-
Notifications
You must be signed in to change notification settings - Fork 1k
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 cirq.eject_z
transformer to replace cirq.EjectZ
#4955
Conversation
tanujkhattar
commented
Feb 4, 2022
- Part of Organization (and deprecation) of cirq-core/optimizers in cirq-core/transformers #4722
- Follows the new Transformer API Compiling: Circuit Transformers API #4483
- Supports no compile tags NoCompile Tag for optimizers NoCompile Tag for optimizers #4253
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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.
One docs nit and one question for my own understanding. Looking good!
@@ -664,7 +664,7 @@ | |||
"id": "oKcnKYTE-ms3" | |||
}, | |||
"source": [ | |||
"You can also use the `cirq.EjectZ` optimizer to attempt to push `cirq.Z` gates towards the end of the circuit." | |||
"You can also use the `cirq.eject_z` optimizer to attempt to push `cirq.Z` gates towards the end of the circuit." |
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.
Should optimzer
here be transformer
?
Similarly in other docs.
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.
You are right, but updating the docs correctly would require a larger change. (for example: transformers are now moment preserving, support no compile tags and automated logging - none of this is already covered).
I've created a separate issue to track that docs are updated correctly once the migration is complete -- #4960 and would prefer to make this change (s/optimizer/transformer) there.
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.
SGTM.
qubit_phase[q] = 0 | ||
if not (key or single_qubit_decompositions.is_negligible_turn(p, atol)): | ||
yield ops.Z(q) ** (p * 2) | ||
elif key: |
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.
It looks like this key-checking block replaces the if moment_index is not None
block in the old code, handling PhasedXZ tracking. Is that correct?
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.
Yes, that's correct.
The idea here is that if the previous operation acting on the qubit q
is a PhasedXZGate
then instead of dumping a new Z ** (2 * p)
gate on q
, we simply modify the previous z exponent of the previous PhasedXZGate
.