-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 qpy custom ControlledGate with overloaded _define() #8927
Conversation
This commit fixes a bug in the QPY serialization of ControlledGate subclasses that defined custom _define() methods. The _define() method is the typical way to provide a custom definition in Gate classes. While ControlledGate class provides an alternative interface that handles custom control states at initialization, but the _define() interface is still valid even if it doesn't account for this. In Qiskit#8571 we updated the QPY serialization code to use the _definition() method directly to correctly handle the open control case. But this fix neglected the case where people were providing definitions via the mechanism from Gate. This commit fixes this assumption by ensuring we load the definition that comes from _define() which will fix the serialization of these custom subclasses. Fixes Qiskit#8794
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:
|
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.
Looks good, thanks!
Pull Request Test Coverage Report for Build 3269159391
💛 - Coveralls |
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.
Fixed merge conflict in tests (just because #8928 also appended tests to the end of the file). Reapproving.
This commit fixes a bug in the QPY serialization of ControlledGate subclasses that defined custom _define() methods. The _define() method is the typical way to provide a custom definition in Gate classes. While ControlledGate class provides an alternative interface that handles custom control states at initialization, but the _define() interface is still valid even if it doesn't account for this. In #8571 we updated the QPY serialization code to use the _definition() method directly to correctly handle the open control case. But this fix neglected the case where people were providing definitions via the mechanism from Gate. This commit fixes this assumption by ensuring we load the definition that comes from _define() which will fix the serialization of these custom subclasses. Fixes #8794 Co-authored-by: Jake Lishman <jake.lishman@ibm.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 3fb8939)
This commit fixes a bug in the QPY serialization of ControlledGate subclasses that defined custom _define() methods. The _define() method is the typical way to provide a custom definition in Gate classes. While ControlledGate class provides an alternative interface that handles custom control states at initialization, but the _define() interface is still valid even if it doesn't account for this. In Qiskit#8571 we updated the QPY serialization code to use the _definition() method directly to correctly handle the open control case. But this fix neglected the case where people were providing definitions via the mechanism from Gate. This commit fixes this assumption by ensuring we load the definition that comes from _define() which will fix the serialization of these custom subclasses. Fixes Qiskit#8794 Co-authored-by: Jake Lishman <jake.lishman@ibm.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
This commit fixes a bug in the QPY serialization of ControlledGate subclasses that defined custom _define() methods. The _define() method is the typical way to provide a custom definition in Gate classes. While ControlledGate class provides an alternative interface that handles custom control states at initialization, but the _define() interface is still valid even if it doesn't account for this. In #8571 we updated the QPY serialization code to use the _definition() method directly to correctly handle the open control case. But this fix neglected the case where people were providing definitions via the mechanism from Gate. This commit fixes this assumption by ensuring we load the definition that comes from _define() which will fix the serialization of these custom subclasses. Fixes #8794 Co-authored-by: Jake Lishman <jake.lishman@ibm.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 3fb8939) Co-authored-by: Matthew Treinish <mtreinish@kortar.org> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Summary
This commit fixes a bug in the QPY serialization of ControlledGate subclasses that defined custom _define() methods. The _define() method is the typical way to provide a custom definition in Gate classes. While ControlledGate class provides an alternative interface that handles custom control states at initialization, but the _define() interface is still valid even if it doesn't account for this. In #8571 we updated the QPY serialization code to use the _definition() method directly to correctly handle the open control case. But this fix neglected the case where people were providing definitions via the mechanism from Gate. This commit fixes this assumption by ensuring we load the definition that comes from _define() which will fix the serialization of these custom subclasses.
Details and comments
Fixes #8794