-
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
Add RemoveIdentityEquivalent transpiler pass #12384
Conversation
Pull Request Test Coverage Report for Build 9038654167Warning: 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 |
28521ea
to
e442e7a
Compare
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 having a pass like this in the library makes a lot of sense. We don't want to run it by default, but if you know it makes sense in your use case then having this available is useful. We could even look at adding it to the end of the init stage in the preset pass managers and tying the atol
value to the approximation_degree
value somehow.
I like the general idea to drop negligible gates but think this implementation is not optimal. |
The concern I have with just relying on the |
|
3718d9a
to
2205d32
Compare
2205d32
to
9f57938
Compare
try:
decomp = TwoQubitWeylDecomposition(node.op, fidelity=fid, _specialization=_specializations.IdEquiv)
except QiskitError:
pass
else:
<replace dag node with 1q gates from decomp> |
Doing linear algebra is too expensive when the aim is just to drop gates with small angles. |
This pivots the pass to work for all gates by checking all the parameters in the standard gate library are within the specified tolerance, and the matrix is equivalent to an identity for any other gate defined in Python (with a matrix defined). To improve the performance of the pass it is written in rust now. Additionally the class is named to RemoveIdentityEquivalent to make the purpose of the pass slightly more clear.
One or more of the following people are relevant to this code:
|
I've reworked this PR to compute the identity equivalence numerically for all gates with a matrix defined and to operate purely in Rust for performance. While these gates would be likely caught in later stages between the synthesis peephole optimization and/or The only todo here is to expand the test coverage which I need to expand to cover all the combinations of inputs in the pass now. |
Pull Request Test Coverage Report for Build 11559067940Details
💛 - 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.
Some minor comments, otherwise LGTM 🙂
Co-authored-by: Julien Gacon <[email protected]>
Co-authored-by: Julien Gacon <[email protected]>
Summary
This adds a new transpiler pass
RemoveIdentityEquivalent
, which is used to remove anyGate
in a circuit that isequivalent to identity and doesn't have an impact on the circuit. This pass works for any gate that defines a matrix and only those that have numeric parameters (a symbolic parameter precludes having a matrix). It computes the average gate fidelity of the the operation for an identity target and if it's below the the specified tolerance the gate will be removed.
Details and comments
Fixes #8654
Co-authored-by: Matthew Treinish [email protected]
TODO: