-
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 extraction of controlled parametric gates #13067
Fix extraction of controlled parametric gates #13067
Conversation
The `mutable` check in the controlled-gate `OperationFromPython` extraction logic to check for a mutated `base_gate` was overzealous, and would return false positives for parametric controlled gates. The only modification to `base_gate` of a standard-library gate that would not have caused data-model problems from Python space would be setting the base-gate label, which is used for a public feature of the circuit visualisers. The change to `get_standard_gate_name_mapping` is just a minor convenience to make the gate objects directly appendable to a circuit; previously, each `Parameter` object was distinct and had a UUID clash with others of the same name, so could not be used together. The new behaviour is purely a convenience for tests; it largely should not be useful for users to directly append these gates.
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 10665329212Details
💛 - 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.
LGTM, this is a straightforward fix and good to see. This actually fixes some edge cases I was hitting during the development of the DAGCircuit PR around things like CXGate
ended up as a PyGate
in rust incorrectly and complicating equality checks.
The `mutable` check in the controlled-gate `OperationFromPython` extraction logic to check for a mutated `base_gate` was overzealous, and would return false positives for parametric controlled gates. The only modification to `base_gate` of a standard-library gate that would not have caused data-model problems from Python space would be setting the base-gate label, which is used for a public feature of the circuit visualisers. The change to `get_standard_gate_name_mapping` is just a minor convenience to make the gate objects directly appendable to a circuit; previously, each `Parameter` object was distinct and had a UUID clash with others of the same name, so could not be used together. The new behaviour is purely a convenience for tests; it largely should not be useful for users to directly append these gates. (cherry picked from commit 8f33084)
The `mutable` check in the controlled-gate `OperationFromPython` extraction logic to check for a mutated `base_gate` was overzealous, and would return false positives for parametric controlled gates. The only modification to `base_gate` of a standard-library gate that would not have caused data-model problems from Python space would be setting the base-gate label, which is used for a public feature of the circuit visualisers. The change to `get_standard_gate_name_mapping` is just a minor convenience to make the gate objects directly appendable to a circuit; previously, each `Parameter` object was distinct and had a UUID clash with others of the same name, so could not be used together. The new behaviour is purely a convenience for tests; it largely should not be useful for users to directly append these gates. (cherry picked from commit 8f33084) Co-authored-by: Jake Lishman <[email protected]>
Summary
The
mutable
check in the controlled-gateOperationFromPython
extraction logic to check for a mutatedbase_gate
was overzealous, and would return false positives for parametric controlled gates. The only modification tobase_gate
of a standard-library gate that would not have caused data-model problems from Python space would be setting the base-gate label, which is used for a public feature of the circuit visualisers.The change to
get_standard_gate_name_mapping
is just a minor convenience to make the gate objects directly appendable to a circuit; previously, eachParameter
object was distinct and had a UUID clash with others of the same name, so could not be used together. The new behaviour is purely a convenience for tests; it largely should not be useful for users to directly append these gates.Details and comments