Skip to content
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

cleanup_operations replaces CZ with CZ**-1 #6428

Closed
NoureldinYosri opened this issue Jan 29, 2024 · 3 comments · Fixed by #6436
Closed

cleanup_operations replaces CZ with CZ**-1 #6428

NoureldinYosri opened this issue Jan 29, 2024 · 3 comments · Fixed by #6436
Assignees
Labels
kind/bug-report Something doesn't seem to work. triage/discuss Needs decision / discussion, bring these up during Cirq Cynque

Comments

@NoureldinYosri
Copy link
Collaborator

NoureldinYosri commented Jan 29, 2024

Description of the issue
cleanup_operations replaced CZ with CZ**-1 where that was unneccassry. in this case the result is still correct since CZ==CZ**-1. but it's worth invisigating whether this a side effec of CZ==CZ**-1 or a symptom of a bug.

Digging deeper into the method the change happens when it calls cirq.transformers.eject_phased_paulis

How to reproduce the issue

The input (a CNOT written in CZ gateset)

c = cirq.Circuit(
         (cirq.Z**0.75).on(cirq.LineQubit(0)) ,
         (cirq.X**-0.25).on(cirq.LineQubit(1)) ,
         (cirq.X**0.5000000000000001).on(cirq.LineQubit(0)) ,
         (cirq.Y**-0.5).on(cirq.LineQubit(1)) ,
         (cirq.S**-1).on(cirq.LineQubit(0)) ,
         (cirq.Y**-0.5).on(cirq.LineQubit(0)) ,
         cirq.CZ(cirq.LineQubit(0), cirq.LineQubit(1)) ,
         (cirq.S**-1).on(cirq.LineQubit(0)) ,
         (cirq.S**-1).on(cirq.LineQubit(1)) ,
         (cirq.Y**0.5).on(cirq.LineQubit(0)) ,
         (cirq.Y**0.5).on(cirq.LineQubit(1)) ,
         (cirq.Y**0.5).on(cirq.LineQubit(0)) ,
         (cirq.X**-0.24999999999999997).on(cirq.LineQubit(1)) ,
         (cirq.Z**-0.75).on(cirq.LineQubit(0)) ,
)
print(cirq.transformers.analytical_decompositions.two_qubit_to_cz.cleanup_operations(c))

result

[cirq.PhasedXPowGate(phase_exponent=-0.5000000000000002, exponent=0.5).on(cirq.LineQubit(1)), (cirq.CZ**-1.0).on(cirq.LineQubit(0), cirq.LineQubit(1)), cirq.PhasedXPowGate(phase_exponent=0.4999999999999998, exponent=0.5).on(cirq.LineQubit(1))]

related to #6422

@NoureldinYosri NoureldinYosri added the kind/bug-report Something doesn't seem to work. label Jan 29, 2024
@NoureldinYosri NoureldinYosri added the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Jan 29, 2024
@eliottrosenberg
Copy link
Collaborator

Thanks, Nour!

@NoureldinYosri
Copy link
Collaborator Author

NoureldinYosri commented Feb 1, 2024

the change happens in

def _single_cross_over_cz(op: ops.Operation, qubit_with_w: 'cirq.Qid') -> 'cirq.OP_TREE':

I don't think this is a bug. the function $f(t) = {CZ}^t$ is periodic with period 2. so if we restrict $t$ to $-1 \leq t \leq 1$ then $f(t) = \overline{f(-t)}$.

Even if I restrict $t$ to that range, deciding between 1 and -1 will be ambiguous. and dropping either end (-1 or 1) is not an option

@NoureldinYosri NoureldinYosri self-assigned this Feb 1, 2024
@NoureldinYosri
Copy link
Collaborator Author

offline update from @eliottrosenberg, it's a bug because in the original example #6422 (comment) partial CZs aren't allowed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug-report Something doesn't seem to work. triage/discuss Needs decision / discussion, bring these up during Cirq Cynque
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants