Skip to content
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 12 commits into from
Mar 30, 2022

Conversation

mtreinish
Copy link
Member

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

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 mtreinish added the Changelog: New Feature Include in the "Added" section of the changelog label Jan 20, 2022
@mtreinish mtreinish added this to the 0.20 milestone Jan 20, 2022
@mtreinish mtreinish requested a review from a team as a code owner January 20, 2022 14:25
jakelishman
jakelishman previously approved these changes Jan 20, 2022
Copy link
Member

@jakelishman jakelishman left a 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.

qiskit/transpiler/passes/utils/gates_basis.py Outdated Show resolved Hide resolved
@jakelishman jakelishman added stable backport potential The bug might be minimal and/or import enough to be port to stable and removed stable backport potential The bug might be minimal and/or import enough to be port to stable labels Jan 20, 2022
@coveralls
Copy link

coveralls commented Jan 20, 2022

Pull Request Test Coverage Report for Build 2067540294

  • 18 of 19 (94.74%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 83.76%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/transpiler/passes/utils/gates_basis.py 18 19 94.74%
Totals Coverage Status
Change from base Build 2067537988: 0.001%
Covered Lines: 53239
Relevant Lines: 63561

💛 - Coveralls

Co-authored-by: John Lapeyre <jlapeyre@users.noreply.github.com>
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.
mtreinish and others added 2 commits January 20, 2022 14:03
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 <jlapeyre@users.noreply.github.com>
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.
Copy link
Contributor

@kevinhartman kevinhartman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@mergify mergify bot merged commit c28fe7b into Qiskit:main Mar 30, 2022
@mtreinish mtreinish deleted the gates_in_basis_target_aware branch March 30, 2022 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants