-
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
Detect duplicates in QuantumCircuit.compose
#11451
Conversation
Previously, `QuantumCircuit.compose` would silently attempt to compose multiple qubits onto the same state, which would fail in weird ways if any multi-qubit instructions were acting on these collapsed qubits.
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 7448349417Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - 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, now the raised error makes a lot more sense 👍🏻
In retrospect, I'll remove this from "stable backport", because the bug's been open for ages without huge complaints, and we're close to 1.0 anyway, so there's little need to increase the risk of the 0.45.2 release. |
After the move of `QuantumCircuit.data` to Rust-space, some of the state modification had started happening before the error checking, which could potentially leave an in-place modification in a partially applied state if an exception triggered during processing. This also ensures that the qubits and clbits arguments are checked, even if the actual data being composed is empty.
The new test here caught a sort-of bug in #11214 where there was an early return for empty |
Nice catch 👍🏻 there seem to be some other modifications to |
The Seems easy enough to change, at any rate - done in 48116a. While it appears like I moved it after some possible return paths, those return paths all (necessarily) call |
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 thanks for the updates!
* Detect duplicates in `QuantumCircuit.compose` Previously, `QuantumCircuit.compose` would silently attempt to compose multiple qubits onto the same state, which would fail in weird ways if any multi-qubit instructions were acting on these collapsed qubits. * Ensure error-checking happens before any modification After the move of `QuantumCircuit.data` to Rust-space, some of the state modification had started happening before the error checking, which could potentially leave an in-place modification in a partially applied state if an exception triggered during processing. This also ensures that the qubits and clbits arguments are checked, even if the actual data being composed is empty. * Delay mutation until the last possible time
Summary
Previously,
QuantumCircuit.compose
would silently attempt to compose multiple qubits onto the same state, which would fail in weird ways if any multi-qubit instructions were acting on these collapsed qubits.Details and comments
Fix #4970.