Fix UnitarySynthesis pass bug when target contains global gates
#13651
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…using target with global gates.
|
One or more of the following people are relevant to this code:
|
ElePT
added
the
Changelog: Bugfix
2 tasks
ElePT
added
the
stable backport potential
mtreinish
reviewed
Jan 10, 2025
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
ElePT
commented
Jan 10, 2025
Pull Request Test Coverage Report for Build 12743897780Details
💛 - Coveralls |
releasenotes/notes/fix-unitary-synthesis-global-gates-19b93840b28cfcf7.yaml
Outdated
Show resolved
Hide resolved
|
|
||
| // Filter out non-2q-gate candidates | ||
| if op.operation.num_qubits() != 2 { | ||
| continue; |
There was a problem hiding this comment.
it's the same behavior both for 1-qubit and n-qubits gates (n>2)?
There was a problem hiding this comment.
yes, the gate matrix needs to be 4x4 in this part of the code, any n!=2 qubit gate causes the dimension mismatch panic
mtreinish
approved these changes
Jan 13, 2025
mergify bot
pushed a commit
that referenced
this pull request
Jan 13, 2025
…3651) * Confirm 2q basis gate candidates are in fact 2-qubit gates. Add test using target with global gates. * Add reno * Apply Matt's suggestion Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Add semicolon * Apply Shelly's suggestion --------- Co-authored-by: Matthew Treinish <mtreinish@kortar.org> (cherry picked from commit d041d5b)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 14, 2025
…3651) (#13656) * Confirm 2q basis gate candidates are in fact 2-qubit gates. Add test using target with global gates. * Add reno * Apply Matt's suggestion Co-authored-by: Matthew Treinish <mtreinish@kortar.org> * Add semicolon * Apply Shelly's suggestion --------- Co-authored-by: Matthew Treinish <mtreinish@kortar.org> (cherry picked from commit d041d5b) Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
While working on #12850 I realized that the
UnitarySynthesisRust implementation would sometimes append non-2q-gates to the basis set and panic over the dimension mismatch. I had assumed thattarget.operation_names_for_qargs(Some(&qubits))would only return gates whose number of qubits matchedlen(qubits), but this assumption is incorrect. It will return any gate with some overlap with the provided qargs, independently of the number of qubits of the gate. This bug would come up often when the target contained global gates.I believe that the fix is as simple as making sure the 2q basis contains only 2q gates, at least for the cases I found.
Details and comments