-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Make GatesInBasis transpiler pass Target aware #7548
Merged
Merged
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
This commit makes the GatesInBasis transpiler pass target aware. It adds a new kwarg, target, which optionally takes a Target object. If it's specified the basis_gates required field will be ignored and the pass will check that the gates and their qargs are valid in the target. Part of Qiskit#7113
mtreinish
commented
Jan 20, 2022
jakelishman
previously approved these changes
Jan 20, 2022
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.
Seems fine as-is - I've approved because I'm not super bothered either way about the comment, so you can merge if you're not in favour.
Pull Request Test Coverage Report for Build 2067540294
💛 - Coveralls |
jlapeyre
reviewed
Jan 20, 2022
Co-authored-by: John Lapeyre <[email protected]>
Now that we can specify a target or basis_gates it's probably best that we don't require basis_gates is always set, especially since a target will always take precedence over the basis_gates list. This commit makes basis_gates optional, but explicitly raises an error if neither basis_gates or target is set on the constructor.
jlapeyre
reviewed
Jan 20, 2022
This commit tweaks the internal loop to be over dag.op_nodes() instead of dag.topological_op_nodes() which was used in earlier iterations. In local benchmarking dag.op_nodes() is on average 2x faster than dag.topological_op_nodes().
This commit improves the performance of the basis_gates code path by combining the basis_gates set with the built-in directives and doing only one set lookup per iteration instead of two. Co-authored-by: John Lapeyre <[email protected]>
c948a75
to
557362f
Compare
jlapeyre
reviewed
Jan 21, 2022
mtreinish
commented
Jan 28, 2022
This commit fixes the handling of ideal operations in a target. If an operation is listed as ideal (where it applies on all qubits and doesn't have any error or duration information) in a target then the instruction properties inner dict is set to `{None: None}` to indicate this. For the gates in basis pass this just means it needs to check if the qubits for a particular instruction are None, and if it is we can treat it as a match.
In Qiskit#7778 a new target method, instruction_supported(), was added to simplify checking if an instruction (operation + qubits) is supported on a target object. This commit switches the GatesInBasis pass to use this internally instead of manually querying the target.
kevinhartman
approved these changes
Mar 30, 2022
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.
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
This commit makes the GatesInBasis transpiler pass target aware. It adds
a new kwarg, target, which optionally takes a Target object. If it's
specified the basis_gates required field will be ignored and the pass
will check that the gates and their qargs are valid in the target.
Details and comments
Part of #7113