Fix ordering bug in fsim decomposition (#2732)

cirq/optimizers/two_qubit_to_fsim.py

@@ -21,7 +21,7 @@
 def decompose_two_qubit_interaction_into_four_fsim_gates_via_b(
         interaction: Union['cirq.Operation', 'cirq.Gate', np.ndarray, Any],
         *,
-        fsim_gate: 'cirq.FSimGate',
+        fsim_gate: Union['cirq.FSimGate', 'cirq.ISwapPowGate'],
         qubits: Sequence['cirq.Qid'] = None) -> 'cirq.Circuit':
     """Decomposes operations into an FSimGate near theta=pi/2, phi=0.
 
@@ -50,9 +50,13 @@ def decompose_two_qubit_interaction_into_four_fsim_gates_via_b(
         list will include four operations of the given fsim gate, various single
         qubit operations, and a global phase operation.
     """
-    if not 3 / 8 * np.pi <= fsim_gate.theta <= 5 / 8 * np.pi:
-        raise ValueError('Must have 3π/8 ≤ fsim_gate.theta ≤ 5π/8')
-    if abs(fsim_gate.phi) > np.pi / 4:
+    if isinstance(fsim_gate, ops.ISwapPowGate):
+        mapped_gate = ops.FSimGate(-fsim_gate.exponent * np.pi / 2, 0)
+    else:
+        mapped_gate = fsim_gate
+    if not 3 / 8 * np.pi <= abs(mapped_gate.theta) <= 5 / 8 * np.pi:
+        raise ValueError('Must have 3π/8 ≤ |fsim_gate.theta| ≤ 5π/8')
+    if abs(mapped_gate.phi) > np.pi / 4:
         raise ValueError('Must have abs(fsim_gate.phi) ≤ π/4')
     if qubits is None:
         if isinstance(interaction, ops.Operation):
@@ -66,7 +70,7 @@ def decompose_two_qubit_interaction_into_four_fsim_gates_via_b(
     result_using_b_gates = _decompose_two_qubit_interaction_into_two_b_gates(
         kak, qubits=qubits)
 
-    b_decomposition = _decompose_b_gate_into_two_fsims(fsim_gate=fsim_gate,
+    b_decomposition = _decompose_b_gate_into_two_fsims(fsim_gate=mapped_gate,
                                                        qubits=qubits)
     result = []
     for op in result_using_b_gates:
@@ -75,7 +79,9 @@ def decompose_two_qubit_interaction_into_four_fsim_gates_via_b(
         else:
             result.append(op)
 
-    circuit = circuits.Circuit(result)
+    circuit = circuits.Circuit(
+        fsim_gate(*op.qubits) if op.gate == mapped_gate else op
+        for op in result)
     merge_single_qubit_gates.MergeSingleQubitGates().optimize_circuit(circuit)
     drop_empty_moments.DropEmptyMoments().optimize_circuit(circuit)
     return circuit
@@ -242,7 +248,8 @@ def _fix_single_qubit_gates_around_kak_interaction(
         A list of operations whose kak decomposition approximately equals the
         desired kak decomposition.
     """
-    actual = linalg.kak_decomposition(circuits.Circuit(operations))
+    actual = linalg.kak_decomposition(
+        circuits.Circuit(operations).unitary(qubit_order=qubits))
 
     def dag(a: np.ndarray) -> np.ndarray:
         return np.transpose(np.conjugate(a))

cirq/optimizers/two_qubit_to_fsim_test.py

@@ -8,27 +8,32 @@
 import cirq
 from cirq.optimizers.two_qubit_to_fsim import (
     _decompose_two_qubit_interaction_into_two_b_gates,
-    _decompose_xx_yy_into_two_fsims_ignoring_single_qubit_ops,
-    _sticky_0_to_1,
-)
+    _decompose_xx_yy_into_two_fsims_ignoring_single_qubit_ops, _sticky_0_to_1,
+    _B)
 
 UNITARY_OBJS = [
     cirq.IdentityGate(2),
     cirq.XX**0.25,
     cirq.CNOT,
+    cirq.CNOT(*cirq.LineQubit.range(2)),
+    cirq.CNOT(*cirq.LineQubit.range(2)[::-1]),
     cirq.ISWAP,
     cirq.SWAP,
     cirq.FSimGate(theta=np.pi / 6, phi=np.pi / 6),
 ] + [cirq.testing.random_unitary(4) for _ in range(5)]
 
 FEASIBLE_FSIM_GATES = [
+    cirq.ISWAP,
+    cirq.google.SYC,
     cirq.FSimGate(np.pi / 2, 0),
+    cirq.FSimGate(-np.pi / 2, 0),
     cirq.FSimGate(np.pi / 2, np.pi / 6),
     cirq.FSimGate(np.pi / 2, -np.pi / 6),
     cirq.FSimGate(5 * np.pi / 9, -np.pi / 6),
     cirq.FSimGate(5 * np.pi / 9, 0),
     cirq.FSimGate(4 * np.pi / 9, -np.pi / 6),
     cirq.FSimGate(4 * np.pi / 9, 0),
+    cirq.FSimGate(-4 * np.pi / 9, 0),
     # Extreme points.
     cirq.FSimGate(np.pi * 3 / 8, -np.pi / 4),
     cirq.FSimGate(np.pi * 5 / 8, -np.pi / 4),
@@ -46,6 +51,8 @@ def test_decompose_two_qubit_interaction_into_two_b_gates(obj: Any):
         _decompose_two_qubit_interaction_into_two_b_gates(
             obj, qubits=cirq.LineQubit.range(2)))
     desired_unitary = obj if isinstance(obj, np.ndarray) else cirq.unitary(obj)
+    for operation in circuit.all_operations():
+        assert len(operation.qubits) < 2 or operation.gate == _B
     assert cirq.approx_eq(cirq.unitary(circuit), desired_unitary, atol=1e-6)
 
 
@@ -71,11 +78,17 @@ def test_decompose_xx_yy_into_two_fsims_ignoring_single_qubit_ops_fail():
                          itertools.product(UNITARY_OBJS, FEASIBLE_FSIM_GATES))
 def test_decompose_two_qubit_interaction_into_four_fsim_gates_via_b(
         obj: Any, fsim_gate: cirq.FSimGate):
+    qubits = (obj.qubits
+              if isinstance(obj, cirq.Operation) else cirq.LineQubit.range(2))
     circuit = cirq.decompose_two_qubit_interaction_into_four_fsim_gates_via_b(
         obj, fsim_gate=fsim_gate)
     desired_unitary = obj if isinstance(obj, np.ndarray) else cirq.unitary(obj)
+    for operation in circuit.all_operations():
+        assert len(operation.qubits) < 2 or operation.gate == fsim_gate
     assert len(circuit) <= 4 + 5
-    assert cirq.approx_eq(cirq.unitary(circuit), desired_unitary, atol=1e-6)
+    assert cirq.approx_eq(circuit.unitary(qubit_order=qubits),
+                          desired_unitary,
+                          atol=1e-6)
 
 
 def test_decompose_two_qubit_interaction_into_four_fsim_gates_via_b_validate():