`X.controlled(cirq.SumOfProducts([[1]]))` does not optimize to `CX`
X.controlled simplifies to CX when possible (and Z to CZ). However this does not work when the control_values is represented as a SumOfProducts, requiring workarounds by any code expecting such optimizations.
A fix for this is provided in #5873.
>>> gate1 = X.controlled(cirq.SumOfProducts([[1]]))
>>> isinstance(gate1, cirq.ControlledGate) # Right now
>>> isinstance(gate1, cirq.CXPowGate) # After the change
This is probably backwards incompatible. Potential considerations:
- LSP is not strictly satisfied because cirq.CXPowGate is not a subclass of cirq.ControlledGate
- However, the behavior of the returned object should be "similar" -- is that true? is that enough?
- Also, expected return type of
X.controlledis acirq.Gate-- so technically it's not a breaking change because the return type is still aGate. And therefore LSP is also satisfied.
One open question:
- Is there any case where controlled gate returned right now would have a significantly different behavior than CXPowGate which would end up breaking existing code. If not, then let's do it!
decompose on X.controlled(cirq.SumOfProducts([[1]])) currently throws TypeError: object of type '<class 'cirq.ops.controlled_gate.ControlledGate'>' does have a _decompose_ method, but it returned NotImplemented or None. So going ahead with this strictly benefits that situation.
Also note that all other cases for X.controlled(...) already return CX when possible. e.g. X.controlled(cirq.ProductOfSums([[1]]) returns CX. Just SumOfProducts([[1]]) doesn't, even though it represents the same control value. So fixing this improves consistency.
https://github.com/quantumlib/Cirq/blob/master/cirq-core/cirq/ops/common_gates_test.py#L125-L128
@tanujkhattar Any further thoughts on this? The linked PR is ready to go. I added additional details in the comment above.