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

Fix circuit.append() to use protocols.commute to verify if two operations can be "crossed" or not. #4882

Closed
tanujkhattar opened this issue Jan 24, 2022 · 1 comment · Fixed by #4872
Labels
area/circuits area/classical/control area/protocols/commutes kind/bug-report Something doesn't seem to work. triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@tanujkhattar
Copy link
Collaborator

Description of the issue
Since #4870 breaks the assumption that two operations acting on disjoint qubits always commute, circuit.append() should use protocols.commute to find the leftmost moment in which an operation can be inserted.

As part of the work done for CCO, this was implemented by adding a special case of checking whether the current operation is a CCO and if yes, whether the previous moment has a measurement with matching control keys.

if moment.operates_on(op_qubits) or (
op_control_keys & protocols.measurement_key_objs(moment)
):
return last_available

This is problematic because it works only in one direction and breaks optimizers which relies on doing operations on the circuit in reverse order.

How to reproduce the issue

>>> import cirq
>>> q =  cirq.LineQubit.range(2)
>>> c = cirq.Circuit(cirq.Moment(cirq.X(q[0])), cirq.Moment(cirq.measure(q[1], key="m")), cirq.Moment(cirq.X(q[0]).with_classical_controls("m")))
>>> print(c)
0: ───X───────X───
              ║
1: ───────M───╫───
          ║   ║
m: ═══════@═══^═══
>>> cirq.AlignLeft().optimize_circuit(c) # Works as expected.
0: ───X───X───
          ║
1: ───M───╫───
      ║   ║
m: ═══@═══^═══
>>> cirq.AlignRight().optimize_circuit(c) # Does not work as expected because implementation depends on inserting operations in the reverse circuit.
>>> print(c) # Wrong output
          ┌──┐
0: ───X────X─────
           ║
1: ────────╫M────
           ║║
m: ════════^@════
          └──┘

Cirq version
0.14.0.dev

@95-martin-orion
Copy link
Collaborator

Cirq sync: we support the general idea here (ensure append works in both directions), but we need to be careful about using protocols.commute for it. For example, appending X(q1) after CX(q0, q1) should keep the X after the CX, even though the two commute.

@95-martin-orion 95-martin-orion added the triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on label Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/circuits area/classical/control area/protocols/commutes kind/bug-report Something doesn't seem to work. triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants