Inconsistencies in using ControlledGate(s) and ControlledOperation(s)

[tanujkhattar]

We currently have ControlledGate and ControlledOperation as generic types which are useful to represent controlled versions of an underlying sub_type. For many common gates, we also have explicit types defined for controlled versions of those gates (eg: X, CX, CCX etc.).

The general pattern to get a controlled_gate from a gate is to call gate.controlled() and to get a controlled_operation from an operation, one should call operation.controlled_by().

There are a few inconsistencies in user experience while dealing with these types, which should be fixed. Some examples are as follows:

Equality of ControlledOperations with explicitly defined controlled types as subtype

q = cirq.LineQubit.range(5)
op = cirq.X(q[0]).controlled_by(*q[1:4]) # Way-1: gate.on().controlled_by()
gate = cirq.X.controlled(4) # Way-2: gate.controlled().on()
op1 = gate(*q[1:5], q[0]) # op1 & op2 should be same, but are not.
op2 = gate(*q[5:0:-1], q[0])
print(repr(gate), repr(op), sep = '\n')
print(op1, op2, op1 == op2)

[output]: 
cirq.ControlledGate(sub_gate=cirq.TOFFOLI, num_controls=2)
cirq.ControlledOperation(sub_operation=cirq.X(cirq.LineQubit(0)),control_values=((1,), (1,), (1,)),controls=(cirq.LineQubit(1), cirq.LineQubit(2), cirq.LineQubit(3)))
CCTOFFOLI(1, 2, 3, 4, 0) CCTOFFOLI(4, 3, 2, 1, 0) False

Issue: Both op1 and op2 are same operations (control on qubits 1, 2, 3, 4 and target on 0), but the equality tests false because their subtypes & controls are different (TOFFOLI(3, 4, 0) vs TOFFOLI(2, 1, 0)).

Proposed Solution: If num_controls > 2, cirq.X.controlled() should return a cirq.ControlledGate(sub_gate = 'X', num_controls = num_controls) instead of first converting to a TOFFOLI. Similar changes to should be done for other gates like Z, CZ, CX etc.

gate.controlled().on() and gate.on().controlled_by() should return the same operations.

q = cirq.LineQubit.range(3)
op1 = cirq.X.controlled(2).on(*q[1:3], q[0]) # Way-1: gate.controlled().on()
op2 = cirq.X(q[0]).controlled_by(*q[1:3]) # Way-2: gate.on().controlled_by()
print(repr(op1), repr(op2), op1 == op2, sep = '\n')

[output]: 
cirq.TOFFOLI(cirq.LineQubit(1), cirq.LineQubit(2), cirq.LineQubit(0))
cirq.ControlledOperation(sub_operation=cirq.X(cirq.LineQubit(0)),control_values=((1,), (1,)),controls=(cirq.LineQubit(1), cirq.LineQubit(2)))
False

Issue: Both op1 and op2 are the same operations, but their types are different. We have custom overrides for gate.controlled for many common gates to return specialized versions of the controlled gate, but these overrides are ignored when we call controlled_by from the corresponding operation.

Proposed Solution: GateOperation should override the controlled_by function to first construct the controlled_gate via gate.controlled() and then apply this controlled_gate on qubits to get a controlled_operation.

Controlled Gates / Ops which have specific types defined should always use/return them by default

It would be nice if can make it harder for instances like cirq.ControlledGate(sub_gate = cirq.X, num_controls = 1) to exist, and always have cirq.CNOT in it's place. Maybe by constructing the controlled gates / ops via a factory that always forwards the requests to gate.controlled / op.controlled_by ?

Update: Part of #3242

[balopat]

Discussed on Cirq Cynque:

• the issues are definitely valid - in fact we should add tests to the standard gate consistency tests that ensure that
  • ordering of the qubits doesn't matter --- e.g. TOFFOLI(a,b,c).controlled_by(d) == TOFFOLI(d,b,c).controlled_by(a)
  • controlled operations created from gates or operations should end up equivalent, meaning TODO (types? python equality?)
• opinions/ideas:
  • cirq.ControlledGate() should be allowed to create "weird" / "non-canonical" gates
  • maybe we should change the definition of equality on these types somehow?

[cduck]

I have a related point that may or may not make sense to be part of #4167.

I consider GlobalPhase to be part of the C*Z family and would expect cirq.GlobalPhaseOperation(exp(1j*pi*ht).controlled_by(q) to be equal to cirq.Z(q)**ht. Should GlobalPhaseOperation also be special-cased?

op = cirq.GlobalPhaseOperation(-1).controlled_by(cirq.LineQubit(0))
assert np.allclose(cirq.unitary(op), cirq.unitary(cirq.Z))
print(op)  # C(0, -1)
print(type(op))  # cirq.ControlledOperation