-
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 bugs with VF2Layout pass and Qiskit Aer 0.13 #11585
Conversation
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 9426638631Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - 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.
I think this is fine as a workaround for this issue. It really is a weird edge case where you're giving it a target which doesn't have any constraints and then specifying with a lower priority argument the constraints to use. But that being said I think is probably safe from an interface perspective and won't have any backwards compatibility implications that I can think of.
The only things that I think are missing here are a unittest or two to cover this case and a release note to document the fix as it is an extension of the API semantics on the pass.
This also seems to affect stable/0.46. |
@Mergifyio backport stable/0.46 stable/1.1 |
✅ Backports have been created
|
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.
I think this would be a good fix to include in the upcoming 1.1.1 release. @chriseclectic do you have time to add the required unit tests and release note within the next week/couple of weeks?
|
Co-authored-by: Matthew Treinish <[email protected]>
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, I left a small suggestion for the reno.
Co-authored-by: Elena Peña Tapia <[email protected]>
* Fix bugs with V2FLayout pass and Qiskit Aer 0.13 * Update qiskit/transpiler/passes/layout/vf2_layout.py Co-authored-by: Matthew Treinish <[email protected]> * test * Update releasenotes/notes/fix-vf2-aer-a7306ce07ea81700.yaml Co-authored-by: Elena Peña Tapia <[email protected]> * long lines --------- Co-authored-by: Luciano Bello <[email protected]> Co-authored-by: Matthew Treinish <[email protected]> Co-authored-by: Elena Peña Tapia <[email protected]> (cherry picked from commit 2b847b8)
* Fix bugs with V2FLayout pass and Qiskit Aer 0.13 * Update qiskit/transpiler/passes/layout/vf2_layout.py Co-authored-by: Matthew Treinish <[email protected]> * test * Update releasenotes/notes/fix-vf2-aer-a7306ce07ea81700.yaml Co-authored-by: Elena Peña Tapia <[email protected]> * long lines --------- Co-authored-by: Luciano Bello <[email protected]> Co-authored-by: Matthew Treinish <[email protected]> Co-authored-by: Elena Peña Tapia <[email protected]> (cherry picked from commit 2b847b8)
* Fix bugs with V2FLayout pass and Qiskit Aer 0.13 * Update qiskit/transpiler/passes/layout/vf2_layout.py Co-authored-by: Matthew Treinish <[email protected]> * test * Update releasenotes/notes/fix-vf2-aer-a7306ce07ea81700.yaml Co-authored-by: Elena Peña Tapia <[email protected]> * long lines --------- Co-authored-by: Luciano Bello <[email protected]> Co-authored-by: Matthew Treinish <[email protected]> Co-authored-by: Elena Peña Tapia <[email protected]> (cherry picked from commit 2b847b8) Co-authored-by: Christopher J. Wood <[email protected]>
* Fix bugs with V2FLayout pass and Qiskit Aer 0.13 * Update qiskit/transpiler/passes/layout/vf2_layout.py Co-authored-by: Matthew Treinish <[email protected]> * test * Update releasenotes/notes/fix-vf2-aer-a7306ce07ea81700.yaml Co-authored-by: Elena Peña Tapia <[email protected]> * long lines --------- Co-authored-by: Luciano Bello <[email protected]> Co-authored-by: Matthew Treinish <[email protected]> Co-authored-by: Elena Peña Tapia <[email protected]> (cherry picked from commit 2b847b8) Co-authored-by: Christopher J. Wood <[email protected]>
* Fix bugs with V2FLayout pass and Qiskit Aer 0.13 * Update qiskit/transpiler/passes/layout/vf2_layout.py Co-authored-by: Matthew Treinish <[email protected]> * test * Update releasenotes/notes/fix-vf2-aer-a7306ce07ea81700.yaml Co-authored-by: Elena Peña Tapia <[email protected]> * long lines --------- Co-authored-by: Luciano Bello <[email protected]> Co-authored-by: Matthew Treinish <[email protected]> Co-authored-by: Elena Peña Tapia <[email protected]>
Summary
Qiskit Aer 0.13 is incompatible with the pass manager generated by
generate_preset_pass_manager
due to bugs with how VF2Layout pass handles its Target, which does not contain a coupling map or qargs.Details and comments
This fixes these so that VF2 will only use the target generated coupling map if it is not None, and doesn't error from trying to iterate over
target.qargs
that are also None in these cases.