Add `cirq.eject_phased_paulis` transformer to replace `cirq.EjectPhasedPaulis` #4958

tanujkhattar · 2022-02-07T08:25:31Z

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

Breaking change: cirq.EjectPhasedPaulis optimizer will now remove empty moments by default.

…aulis

review-notebook-app · 2022-02-07T08:25:35Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

MichaelBroughton

A few nits and then two bigger feedback points:

We should probably have this work with PhasedXZ
Is there a change in behavior on the old transformer that we should fix ?

cirq-core/cirq/optimizers/eject_phased_paulis.py

cirq-core/cirq/optimizers/eject_phased_paulis_test.py

cirq-core/cirq/transformers/analytical_decompositions/two_qubit_to_cz.py

cirq-core/cirq/transformers/eject_phased_paulis.py

MichaelBroughton · 2022-02-07T18:58:46Z

cirq-core/cirq/transformers/eject_phased_paulis.py

+    atol: float = 1e-8,
+    eject_parameterized: bool = False,
+) -> 'cirq.Circuit':
+    """Transformer pass to push X, Y, and PhasedX gates towards the end of the circuit.


This can probably be made to include PhasedXZ gates as well.

Added support for PhasedXZ gates with z_exponent = 0 (which are equivalent to PhasedX gates). Added a test which demonstrates how to deal with a general PhasedXZGate, i.e.
run cirq.eject_z(cirq.eject_phased_paulis(cirq.eject_z(c))).

cirq.eject_z(c) : This ejects Z gates and ensures that the z_component of PhasedXZGate is 0.

cirq.eject_phased_paulis(c): This ejects PhasedXPowGates, potentially leaving z phased from CZ phase kickback, which will then be removed by the 3rd cirq.eject_z().

Awesome, could we also support the case where axis_exponent=0 and x_exponent=0 but z_exponent can be whatever we want ? This seems like it might be able to be worked into _try_get_known_z_half_turns without much difficulty ?

cirq-core/cirq/transformers/eject_phased_paulis_test.py

…t_phased_pauli

tanujkhattar · 2022-02-07T23:21:21Z

@MichaelBroughton Addressed Feedback, ready for another look.

MichaelBroughton

LGTM, two concerns now are:

Can we accomodate the PhasedXZ case where axis_exponent=0 and x_xeponent=0 (aka just a Z).
Can we double check that CCO behave themselves when they are the operations that are getting moved (it's good that things work when they aren't getting moved) ?

MichaelBroughton · 2022-02-08T00:08:58Z

cirq-core/cirq/transformers/eject_phased_paulis.py

+    atol: float = 1e-8,
+    eject_parameterized: bool = False,
+) -> 'cirq.Circuit':
+    """Transformer pass to push X, Y, and PhasedX gates towards the end of the circuit.


Awesome, could we also support the case where axis_exponent=0 and x_exponent=0 but z_exponent can be whatever we want ? This seems like it might be able to be worked into _try_get_known_z_half_turns without much difficulty ?

MichaelBroughton · 2022-02-08T00:09:23Z

cirq-core/cirq/transformers/eject_phased_paulis.py

+    atol: float = 1e-8,
+    eject_parameterized: bool = False,
+) -> 'cirq.Circuit':
+    """Transformer pass to push X, Y, and PhasedX gates towards the end of the circuit.


Might want to also mention PhasedXZGates in the topline of the docstring.

MichaelBroughton · 2022-02-08T00:19:33Z

cirq-core/cirq/transformers/eject_phased_paulis_test.py

+            [cirq.PhasedXPowGate(phase_exponent=0.25).on(a)],
+            [cirq.measure(a, key="m")],
+            [cirq.X(b).with_classical_controls("m")],


What would be the expected output for something like ?

[cirq.PhasedXPowGate(phase_exponent=0.25).on(a)], [cirq.measure(a, key="m")], [cirq.X(b).with_classical_controls("m")], [cirq.measure(b, key='m2')]

?

cirq.X(b).with_classical_controls will be unaffected because op.gate is None for CCOs. The first PhasedXPowGate will get absorbed into the measurement. So, the output will be:

a: ───!M───────────────── ║ b: ───╫────X───M('m2')─── ║ ║ m: ═══@════^═════════════

MichaelBroughton · 2022-02-08T00:20:26Z

cirq-core/cirq/transformers/eject_phased_paulis.py

+    circuit: 'cirq.AbstractCircuit',
+    *,
+    context: Optional['cirq.TransformerContext'] = None,
+    atol: float = 1e-8,


This seems a little tight, floats are usually only good up to 1e-6 or so.

1e-8 tolerance is standard across the codebase, try running grep -RI '1e-8' --include \*.py.

tanujkhattar · 2022-02-08T04:01:45Z

Added support for PhasedXZ gates with only z_exponent as the non-zero angle. Also updated docstrings.
CCO's will not get moved because op.gate is None when op is a CCO, so the operation would be an "unknown" operation and will act as a blocker. Same is true for other existing transformers as well. Adding support for running transformers on the gates inside CCOs is currently being discussed in Recursive transformer / iterator #4923 and I'd prefer to not block this PR for the same.

…edPaulis` (quantumlib#4958) * Add cirq.eject_phased_paulis transformer to replace cirq.EjectPhasedPaulis * Add CCO tests, support PhasedXZGates * Support PhasedXZGates equivalent to z rotations and update docstrings

Add cirq.eject_phased_paulis transformer to replace cirq.EjectPhasedP…

f0e92ba

…aulis

tanujkhattar requested review from cduck, verult, vtomole, wcourtney and a team as code owners February 7, 2022 08:25

CirqBot added the size: XL lines changed >1000 label Feb 7, 2022

tanujkhattar requested a review from mpharrigan February 7, 2022 08:25

MichaelBroughton self-assigned this Feb 7, 2022

MichaelBroughton reviewed Feb 7, 2022

View reviewed changes

tanujkhattar added the BREAKING CHANGE For pull requests that are important to mention in release notes. label Feb 7, 2022

tanujkhattar added 4 commits February 7, 2022 11:57

Merge branch 'master' of https://github.com/quantumlib/cirq into ejec…

1c2882f

…t_phased_pauli

Add CCO tests, support PhasedXZGates

647e31c

Improve docstrings

89f5b1c

Merge branch 'master' of https://github.com/quantumlib/cirq into ejec…

02a8cea

…t_phased_pauli

MichaelBroughton approved these changes Feb 8, 2022

View reviewed changes

Support PhasedXZGates equivalent to z rotations and update docstrings

90d7623

tanujkhattar merged commit c20c99b into quantumlib:master Feb 8, 2022

tanujkhattar deleted the eject_phased_pauli branch February 8, 2022 04:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `cirq.eject_phased_paulis` transformer to replace `cirq.EjectPhasedPaulis` #4958

Add `cirq.eject_phased_paulis` transformer to replace `cirq.EjectPhasedPaulis` #4958

tanujkhattar commented Feb 7, 2022 •

edited

Loading

review-notebook-app bot commented Feb 7, 2022

MichaelBroughton left a comment

MichaelBroughton Feb 7, 2022

tanujkhattar Feb 7, 2022

MichaelBroughton Feb 8, 2022

tanujkhattar commented Feb 7, 2022

MichaelBroughton left a comment

MichaelBroughton Feb 8, 2022

MichaelBroughton Feb 8, 2022

MichaelBroughton Feb 8, 2022

tanujkhattar Feb 8, 2022 •

edited

Loading

MichaelBroughton Feb 8, 2022

tanujkhattar Feb 8, 2022

tanujkhattar commented Feb 8, 2022

Add cirq.eject_phased_paulis transformer to replace cirq.EjectPhasedPaulis #4958

Add cirq.eject_phased_paulis transformer to replace cirq.EjectPhasedPaulis #4958

Conversation

tanujkhattar commented Feb 7, 2022 • edited Loading

review-notebook-app bot commented Feb 7, 2022

MichaelBroughton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tanujkhattar commented Feb 7, 2022

MichaelBroughton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tanujkhattar Feb 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tanujkhattar commented Feb 8, 2022

Add `cirq.eject_phased_paulis` transformer to replace `cirq.EjectPhasedPaulis` #4958

Add `cirq.eject_phased_paulis` transformer to replace `cirq.EjectPhasedPaulis` #4958

tanujkhattar commented Feb 7, 2022 •

edited

Loading

tanujkhattar Feb 8, 2022 •

edited

Loading