-
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 identity removal pass to presets #13363
Conversation
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 11712056331Details
💛 - Coveralls |
This commit adds the newly added RemoveIdentityEquivalent transpiler pass to the preset pass managers for optimization levels 2 and 3. The pass is added to the init and optimization stages. For the init stage the pass is run after 3 or more qubit decomposition. While the pass works fine with 3 qubit gates, the thinking behind this placement was it seems less likely for a multi qubit identity equivalent gate to be added while components of it's decomposition are more likely to be equivalent to an identity. For the optimization stage it is added to the optimization loop. One test needed to be updated because the additional pass changes the decomposition outcome in that test. Although the output circuit is different it is equivalent to the other decomposition and the original circuit. A unitary equivalence check is added to the test to verify that.
be36b59
to
360d625
Compare
@@ -157,6 +158,13 @@ def pass_manager(self, pass_manager_config, optimization_level=None) -> PassMana | |||
if pass_manager_config.routing_method != "none": | |||
init.append(ElidePermutations()) | |||
init.append(RemoveDiagonalGatesBeforeMeasure()) | |||
# Target not set on RemoveIdentityEquivalent because we haven't applied a Layout |
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 I would expect the transpiler to remove Ry(0)
& co even as a very basic optimization, should we also include this target-free run in optimization level 1?
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.
My concern with doing it for level 1 was more about small angles like Ry(1e-8)
will be removed and I was worried that might be surprising to people especially at level 1. But I don't feel super strongly about it, it's easy enough to add it to level 1 if you think that's better.
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.
Hm you're right, I didn't think that these angles are already removed already... as rule we should maybe not increase the level of approximation that is already happening (e.g. cutoffs that unitary re-synthesis does). On level 1 we don't do re-synthesis, right? So then I agree we shouldn't add it one 1 yet 🙂
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.
Level 1 will run 1q re-synthesis, but via Optikize1qGatesDecomposition
not UnitarySynthesis
. I can't remember how that pass handles tolerances, I'll have to check.
expected.p(np.pi - 0.2, 0) | ||
expected.sx(0) | ||
else: | ||
expected = QuantumCircuit(1, global_phase=(15 * np.pi - 1) / 10) |
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.
This is slightly unexpected, I guess at some point a gate is now removed and Optimize1qGatesDecomposition
optimizes the sequence differently? Or why do you think this happens?
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.
That was my assumption, I don't remember the exact details though. Let me run it with a callback and see what's going on exactly.
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.
Looking through each pass it looks like RemoveIdentityEquivalent
changes:
global phase: 4.6124
┌──────┐┌────┐┌───────────┐┌────┐┌───────┐
q: ┤ P(0) ├┤ √X ├┤ P(3.3416) ├┤ √X ├┤ P(3π) ├
└──────┘└────┘└───────────┘└────┘└───────┘
to:
global phase: 4.6124
┌────┐┌───────────┐┌────┐┌───────┐
q: ┤ √X ├┤ P(3.3416) ├┤ √X ├┤ P(3π) ├
└────┘└───────────┘└────┘└───────┘
This changes the output of Optimize1qGatesDecomposition
which runs immediately after from:
global phase: 4.8124
┌───────┐┌────┐┌───────────┐┌────┐
q: ┤ P(-π) ├┤ √X ├┤ P(2.9416) ├┤ √X ├
└───────┘└────┘└───────────┘└────┘
which is the output on calling the pass on the circuit with P(0)
to:
global phase: 4.6124
┌────┐┌───────────┐┌────┐┌───────┐
q: ┤ √X ├┤ P(3.3416) ├┤ √X ├┤ P(3π) ├
└────┘└───────────┘└────┘└───────┘
which is the pass's output with P(0)
removed. What this is coming down to is the preference for Optimize1qGatesDecomposition
to defer to the input circuit if it doesn't think the synthesis output is any better. Since the gate counts of either output are the same, the pass views them as equivalent (in the absence of error rates in the target) and prefer to not modify the circuit instead of use the synthesis output. While in the case of without the removal pass being run there is an extra 1q gate in the sequence the pass views the synthesis output as being more efficient because it has 1 fewer gate.
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.
Ahh that makes sense -- thanks for checking, better to be sure! 🙂
Summary
This commit adds the newly added RemoveIdentityEquivalent transpiler
pass to the preset pass managers for optimization levels 2 and 3. The
pass is added to the init and optimization stages. For the init stage
the pass is run after 3 or more qubit decomposition. While the pass
works fine with 3 qubit gates, the thinking behind this placement was
it seems less likely for a multi qubit identity equivalent gate to be
added while components of it's decomposition are more likely to be
equivalent to an identity. For the optimization stage it is added to
the optimization loop.
One test needed to be updated because the additional pass changes the
decomposition outcome in that test. Although the output circuit is
different it is equivalent to the other decomposition and the original
circuit. A unitary equivalence check is added to the test to verify
that.
Details and comments
This PR is based on top of #12384 and will need to be rebased after it merges. You can see the contents of this PR by looking at the head commit:be36b59