Implement __eq__ for remaining standard-library gates except mcx
#13327
Conversation
This file contains hidden or 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
Most standard-library gates had a specialised `__eq__` added in Qiskitgh-11370, but a small number were left over. This adds specialisation to almost all the stragglers, which were mostly old soft-deperecated gates. Despite not appearing in most circuits, these gates have an outsized effect on equality comparisons if they ever _do_ appear, because they involve construction of entire `definition` fields.
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 11351094849Details
💛 - Coveralls |
Cryoris
reviewed
Oct 15, 2024
There was a problem hiding this comment.
LGTM 👍🏻
Just two questions:
- Would it make sense to move
__eq__just toGateandControlledGate(with the two special cases of MCPhase/U1 that adds the num_ctrl_qubits check)? - I assume you didn't add it to MCX due to all the different gate flavors, right? If yes then we should remember to add this once the flavors are gone and we just have the
MCXGateblock 🙂
|
The trouble is that these specialised For mcx: yeah, I didn't want to get into the can of worms of all the different variants, and what should be considered |
Cryoris
approved these changes
Oct 15, 2024
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
Most standard-library gates had a specialised
__eq__added in gh-11370, but a small number were left over. This adds specialisation to almost all the stragglers, which were mostly old soft-deperecated gates. Despite not appearing in most circuits, these gates have an outsized effect on equality comparisons if they ever do appear, because they involve construction of entiredefinitionfields.Details and comments
You can see the improvement here in the OQ3 exporter for circuits containing some of the affected gates; it uses gate equality at one part of the symbol lookup. For example, given this circuit:
The time of
qasm3.dumps(circ)on my machine went from about 1.15(2)s to 0.76(2)s with this PR, despite only about 9% of the gates in the circuit being affected by this PR.