-
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
Support target and basis gates in Unroll3qOrMore transpiler pass #7997
Conversation
This commit adds two new constructor arguments to the Unroll3qOrMore transpiler pass, target and basis_gates, which are used to specify a backend target and basis gate list respectively. If these are specified the pass will not decompose any multiqubit operations present in the circuit if they are in the target or basis_gates list. Fixes Qiskit#5518 Related to Qiskit#7812 Part of Qiskit#7113
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:
|
Pull Request Test Coverage Report for Build 2269842804
💛 - Coveralls |
""" | ||
super().__init__() | ||
self.target = target | ||
self.basis_gates = None |
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.
self.basis_gates = None | |
self.basis_gates = set(basis_gates) if basis_gates is not None or target |
Why we need to keep target? Seems like both define __contains__
thus we can simplify. This doesn't block your PR. Just my curiosity.
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.
It's mostly leftover from an earlier local attempt. I originally wanted to use target.instruction_supported(node.name, qargs)
to check that the multiqubit gate is defined on the qubits in the circuit. But the problem with that was when this pass is run before the layout phase we don't actually know which physical device qubits the gate will run on so we can't check that in the target.
I think longer term it makes sense to keep them separate because after #7807 merges we can use instruction_supported()
to check based on gate class and potentially parameterizations too instead of just using the name.
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.
Thanks, this PR looks good to me.
Co-authored-by: Naoki Kanazawa <nkanazawa1989@gmail.com>
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, typo aside.
Summary
This commit adds two new constructor arguments to the Unroll3qOrMore
transpiler pass, target and basis_gates, which are used to specify a
backend target and basis gate list respectively. If these are specified
the pass will not decompose any multiqubit operations present in the
circuit if they are in the target or basis_gates list.
Details and comments
Fixes #5518
Related to #7812
Part of #7113