Skip to content
This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

MES Factory gradient support #1421

Merged
merged 10 commits into from
Nov 11, 2020

Conversation

BryceFuller
Copy link
Contributor

@BryceFuller BryceFuller commented Nov 9, 2020

Summary

Updating the MES Factory methods to support the new gradient framework.
This PR resolves #1398

Details and comments

@BryceFuller BryceFuller changed the title [WIP] MES Factory gradient support MES Factory gradient support Nov 11, 2020
@woodsp-ibm woodsp-ibm added the Changelog: New Feature Include in the Added section of the changelog label Nov 11, 2020
Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing parameters to the factories, such as expectation, all have setter/getters. I think for consistency gradient should have them too.

@BryceFuller
Copy link
Contributor Author

The existing parameters to the factories, such as expectation, all have setter/getters. I think for consistency gradient should have them too.

good catch, thanks

@woodsp-ibm
Copy link
Member

woodsp-ibm commented Nov 11, 2020

In looking at the setters/getters now that you have done these I notice the original ones for initial_point, expectation, optimizer that default to None in factory constructor do not allow show that None is a valid value to set these too, via setter, or one would expect to have None back from getter, ie. are Optional like you just did for gradient. I think it should be valid to set to None so you go can back to default. Either way the getter needs to show Optional. Maybe this should/could be done in a separate PR but since its just typehints maybe it can be added/fixed here?

Update: I created a new issue #1428 to address the above, leaving this as purely the gradient change then.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog: New Feature Include in the Added section of the changelog Chemistry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding gradient support for MES Factory Implementations
3 participants