-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix primitives metadata #9285
Fix primitives metadata #9285
Conversation
Co-authored-by: Julian Schuhmacher <jsc@zurich.ibm.com>
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 3742930238
💛 - Coveralls |
LGTM! I would agree that this is straightforward enough to merge without a dedicated test, but let's hold merging off for a bit in case there are concerns. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. LGTM!
releasenotes/notes/fix-primitives-metadata-1e79604126e26b53.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/fix-primitives-metadata-1e79604126e26b53.yaml
Outdated
Show resolved
Hide resolved
* Fix bug metadata Co-authored-by: Julian Schuhmacher <jsc@zurich.ibm.com> * Add reno * Re-trigger CI * Fix typo Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com> Co-authored-by: Julian Schuhmacher <jsc@zurich.ibm.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Julien Gacon <gaconju@gmail.com> Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
Summary
jschuhmac developed a custom estimator for his research project where additional metadata is saved, and realized that all values were updated with each iteration because the list of dicts was instantiated by reference. This PR fixes the reference
Sampler
andEstimator
classes to avoid this problem.Details and comments
Given that the only metadata currently saved are
shots
, and they don't change between circuits, it is not possible to add a unit test with this implementation, but I think this fix is good to have in case anyone wants to extend these classes and save different metadata.