Fix merge_interactions failing to convert into xmon gates (#696)

- Updated the cost metric to include presence-of-non-xmon
- Converting gates to xmon before returning

Fixes #490

Author: Strilanc

cirq/google/merge_interactions.py

@@ -18,17 +18,12 @@
 
 import numpy as np
 
-from cirq import ops
-from cirq.circuits import (
-    Circuit,
-    PointOptimizer,
-    PointOptimizationSummary,
-)
+from cirq import circuits, ops
 from cirq.extension import Extensions
-from cirq.google.decompositions import two_qubit_matrix_to_native_gates
+from cirq.google import decompositions, convert_to_xmon_gates, xmon_gates
 
 
-class MergeInteractions(PointOptimizer):
+class MergeInteractions(circuits.PointOptimizer):
     """Combines series of adjacent one and two-qubit gates operating on a pair
     of qubits."""
 
@@ -40,74 +35,88 @@ def __init__(self,
         self.allow_partial_czs = allow_partial_czs
         self.extensions = extensions or Extensions()
 
-    def optimization_at(self, circuit, index, op):
+    def optimization_at(self,
+                        circuit: circuits.Circuit,
+                        index: int,
+                        op: ops.Operation
+                        ) -> Optional[circuits.PointOptimizationSummary]:
         if len(op.qubits) != 2:
             return None
 
-        interaction_count, indices, matrix = (
+        old_operations, indices, matrix = (
             self._scan_two_qubit_ops_into_matrix(circuit, index, op.qubits))
-        if interaction_count <= 1:
-            return None
 
         # Find a max-3-cz construction.
-        operations = two_qubit_matrix_to_native_gates(
+        new_operations = decompositions.two_qubit_matrix_to_native_gates(
             op.qubits[0],
             op.qubits[1],
             matrix,
             self.allow_partial_czs,
             self.tolerance)
 
-        # TODO: don't replace if there's no benefit in CZ depth.
+        old_interaction_count = len([old_op for old_op in old_operations
+                                     if len(old_op.qubits) == 2])
+        new_interaction_count = len([new_op for new_op in new_operations
+                                     if len(new_op.qubits) == 2])
+        switch_to_new = False
+        switch_to_new |= new_interaction_count < old_interaction_count
+        switch_to_new |= any(not xmon_gates.XmonGate.is_xmon_op(old_op)
+                             for old_op in old_operations)
+        if not switch_to_new:
+            return None
+
+        converter = convert_to_xmon_gates.ConvertToXmonGates()
+        new_xmon_operations = [converter.convert(new_op)
+                               for new_op in new_operations]
 
-        return PointOptimizationSummary(
+        return circuits.PointOptimizationSummary(
             clear_span=max(indices) + 1 - index,
             clear_qubits=op.qubits,
-            new_operations=operations)
+            new_operations=new_xmon_operations)
 
     def _op_to_matrix(self,
-                      op: ops.Operation,
+                      op: Optional[ops.Operation],
                       qubits: Tuple[ops.QubitId, ...]
-                      ) -> Optional[Tuple[np.ndarray, bool]]:
+                      ) -> Optional[np.ndarray]:
         """Determines the effect of an operation on the given qubits.
 
-        The operation must be a 1-qubit operation on one of the given qubits,
-        or a 2-qubit operation on both of the given qubits. Also, the operation
-        must have a known matrix. Otherwise None is returned.
+        If the operation is a 1-qubit operation on one of the given qubits,
+        or a 2-qubit operation on both of the given qubits, and also the
+        operation has a known matrix, then a matrix is returned. Otherwise None
+        is returned.
 
         Args:
             op: The operation to understand.
             qubits: The qubits we care about. Order determines matrix tensor
                 order.
 
         Returns:
-            None, or else a tuple containing a matrix equivalent to the effect
-            of the operation and a boolean indicating if the operation is a
-            2-qubit interaction.
+            None, or else a matrix equivalent to the effect of the operation.
         """
         q1, q2 = qubits
 
         known = self.extensions.try_cast(ops.KnownMatrix, op)
-        if known is None:
+        if known is None or op is None:
             return None
         m = known.matrix()
 
         if op.qubits == qubits:
-            return m, True
+            return m
         if op.qubits == (q2, q1):
-            return MergeInteractions._flip_kron_order(m), True
+            return MergeInteractions._flip_kron_order(m)
         if op.qubits == (q1,):
-            return np.kron(m, np.eye(2)), False
+            return np.kron(m, np.eye(2))
         if op.qubits == (q2,):
-            return np.kron(np.eye(2), m), False
+            return np.kron(np.eye(2), m)
 
         return None
 
     def _scan_two_qubit_ops_into_matrix(
             self,
-            circuit: Circuit,
+            circuit: circuits.Circuit,
             index: Optional[int],
             qubits: Tuple[ops.QubitId, ...]
-    ) -> Tuple[int, List[int], np.ndarray]:
+    ) -> Tuple[List[ops.Operation], List[int], np.ndarray]:
         """Accumulates operations affecting the given pair of qubits.
 
         The scan terminates when it hits the end of the circuit, finds an
@@ -121,37 +130,36 @@ def _scan_two_qubit_ops_into_matrix(
 
         Returns:
             A tuple containing:
-                0. The number of 2-qubit operations that were scanned.
+                0. The operations.
                 1. The moment indices those operations were on.
                 2. A matrix equivalent to the effect of the scanned operations.
         """
 
         product = np.eye(4, dtype=np.complex128)
-        interaction_count = 0
+        all_operations = []
         touched_indices = []
 
         while index is not None:
-            operations = {circuit.operation_at(q, index) for q in qubits}
+            operations = list({circuit.operation_at(q, index) for q in qubits})
             op_data = [
                 self._op_to_matrix(op, qubits)
                 for op in operations
-                if op
             ]
 
             # Stop at any non-constant or non-local interaction.
             if any(e is None for e in op_data):
                 break
-            present_op_data = cast(List[Tuple[np.ndarray, bool]], op_data)
+            present_ops = [op for op in operations if op]
+            present_op_data = cast(List[np.ndarray], op_data)
 
-            for op_mat, interacts in present_op_data:
+            for op_mat in present_op_data:
                 product = np.dot(op_mat, product)
-                if interacts:
-                    interaction_count += 1
+            all_operations.extend(present_ops)
 
             touched_indices.append(index)
             index = circuit.next_moment_operating_on(qubits, index + 1)
 
-        return interaction_count, touched_indices, product
+        return all_operations, touched_indices, product
 
     @staticmethod
     def _flip_kron_order(mat4x4: np.ndarray) -> np.ndarray:

cirq/google/merge_interactions_test.py

@@ -12,134 +12,140 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from cirq import testing
-from cirq import circuits
-from cirq import ops
-from cirq.google import ExpZGate, MergeInteractions, MergeRotations
-from cirq.value import Symbol
+import cirq
+import cirq.google as cg
 
 
-def assert_optimizes(before, after):
-    opt = MergeInteractions()
-    opt.optimize_circuit(before)
+def assert_optimizes(before: cirq.Circuit, expected: cirq.Circuit):
+    actual = cirq.Circuit(before)
+    opt = cg.MergeInteractions()
+    opt.optimize_circuit(actual)
 
     # Ignore differences that would be caught by follow-up optimizations.
     followup_optimizations = [
-        MergeRotations(),
-        circuits.DropNegligible(),
-        circuits.DropEmptyMoments()
+        cg.MergeRotations(),
+        cg.EjectFullW(),
+        cg.EjectZ(),
+        cirq.DropNegligible(),
+        cirq.DropEmptyMoments()
     ]
     for post in followup_optimizations:
-        post.optimize_circuit(before)
-        post.optimize_circuit(after)
+        post.optimize_circuit(actual)
+        post.optimize_circuit(expected)
 
-    if before != after:
+    if actual != expected:
         # coverage: ignore
         print('ACTUAL')
-        print(before)
+        print(actual)
         print('EXPECTED')
-        print(after)
-    assert before == after
+        print(expected)
+    assert actual == expected
 
 
 def assert_optimization_not_broken(circuit):
     """Check that the unitary matrix for the input circuit is the same (up to
     global phase and rounding error) as the unitary matrix of the optimized
     circuit."""
     u_before = circuit.to_unitary_matrix()
-    MergeInteractions().optimize_circuit(circuit)
+    cg.MergeInteractions().optimize_circuit(circuit)
     u_after = circuit.to_unitary_matrix()
 
-    testing.assert_allclose_up_to_global_phase(u_before, u_after, atol=1e-8)
+    cirq.testing.assert_allclose_up_to_global_phase(
+        u_before, u_after, atol=1e-8)
 
 
 def test_clears_paired_cnot():
-    q0 = ops.QubitId()
-    q1 = ops.QubitId()
+    a, b = cirq.LineQubit.range(2)
     assert_optimizes(
-        before=circuits.Circuit([
-            circuits.Moment([ops.CNOT(q0, q1)]),
-            circuits.Moment([ops.CNOT(q0, q1)]),
+        before=cirq.Circuit([
+            cirq.Moment([cirq.CNOT(a, b)]),
+            cirq.Moment([cirq.CNOT(a, b)]),
         ]),
-        after=circuits.Circuit())
+        expected=cirq.Circuit())
 
 
 def test_ignores_czs_separated_by_parameterized():
-    q0 = ops.QubitId()
-    q1 = ops.QubitId()
+    a, b = cirq.LineQubit.range(2)
     assert_optimizes(
-        before=circuits.Circuit([
-            circuits.Moment([ops.CZ(q0, q1)]),
-            circuits.Moment([ExpZGate(
-                half_turns=Symbol('boo'))(q0)]),
-            circuits.Moment([ops.CZ(q0, q1)]),
+        before=cirq.Circuit([
+            cirq.Moment([cirq.CZ(a, b)]),
+            cirq.Moment([cg.ExpZGate(
+                half_turns=cirq.Symbol('boo'))(a)]),
+            cirq.Moment([cirq.CZ(a, b)]),
         ]),
-        after=circuits.Circuit([
-            circuits.Moment([ops.CZ(q0, q1)]),
-            circuits.Moment([ExpZGate(
-                half_turns=Symbol('boo'))(q0)]),
-            circuits.Moment([ops.CZ(q0, q1)]),
+        expected=cirq.Circuit([
+            cirq.Moment([cirq.CZ(a, b)]),
+            cirq.Moment([cg.ExpZGate(
+                half_turns=cirq.Symbol('boo'))(a)]),
+            cirq.Moment([cirq.CZ(a, b)]),
         ]))
 
 
 def test_ignores_czs_separated_by_outer_cz():
-    q00 = ops.QubitId()
-    q01 = ops.QubitId()
-    q10 = ops.QubitId()
+    q00 = cirq.GridQubit(0, 0)
+    q01 = cirq.GridQubit(0, 1)
+    q10 = cirq.GridQubit(1, 0)
     assert_optimizes(
-        before=circuits.Circuit([
-            circuits.Moment([ops.CZ(q00, q01)]),
-            circuits.Moment([ops.CZ(q00, q10)]),
-            circuits.Moment([ops.CZ(q00, q01)]),
+        before=cirq.Circuit([
+            cirq.Moment([cirq.CZ(q00, q01)]),
+            cirq.Moment([cirq.CZ(q00, q10)]),
+            cirq.Moment([cirq.CZ(q00, q01)]),
         ]),
-        after=circuits.Circuit([
-            circuits.Moment([ops.CZ(q00, q01)]),
-            circuits.Moment([ops.CZ(q00, q10)]),
-            circuits.Moment([ops.CZ(q00, q01)]),
+        expected=cirq.Circuit([
+            cirq.Moment([cirq.CZ(q00, q01)]),
+            cirq.Moment([cirq.CZ(q00, q10)]),
+            cirq.Moment([cirq.CZ(q00, q01)]),
         ]))
 
 
 def test_cnots_separated_by_single_gates_correct():
-    q0 = ops.QubitId()
-    q1 = ops.QubitId()
+    a, b = cirq.LineQubit.range(2)
     assert_optimization_not_broken(
-        circuits.Circuit.from_ops(
-            ops.CNOT(q0, q1),
-            ops.H(q1),
-            ops.CNOT(q0, q1),
+        cirq.Circuit.from_ops(
+            cirq.CNOT(a, b),
+            cirq.H(b),
+            cirq.CNOT(a, b),
         ))
 
 
 def test_czs_separated_by_single_gates_correct():
-    q0 = ops.QubitId()
-    q1 = ops.QubitId()
+    a, b = cirq.LineQubit.range(2)
     assert_optimization_not_broken(
-        circuits.Circuit.from_ops(
-            ops.CZ(q0, q1),
-            ops.X(q1),
-            ops.X(q1),
-            ops.X(q1),
-            ops.CZ(q0, q1),
+        cirq.Circuit.from_ops(
+            cirq.CZ(a, b),
+            cirq.X(b),
+            cirq.X(b),
+            cirq.X(b),
+            cirq.CZ(a, b),
         ))
 
 
 def test_inefficient_circuit_correct():
     t = 0.1
     v = 0.11
-    q0 = ops.QubitId()
-    q1 = ops.QubitId()
+    a, b = cirq.LineQubit.range(2)
     assert_optimization_not_broken(
-        circuits.Circuit.from_ops(
-            ops.H(q1),
-            ops.CNOT(q0, q1),
-            ops.H(q1),
-            ops.CNOT(q0, q1),
-            ops.CNOT(q1, q0),
-            ops.H(q0),
-            ops.CNOT(q0, q1),
-            ops.Z(q0)**t, ops.Z(q1)**-t,
-            ops.CNOT(q0, q1),
-            ops.H(q0), ops.Z(q1)**v,
-            ops.CNOT(q0, q1),
-            ops.Z(q0)**-v, ops.Z(q1)**-v,
+        cirq.Circuit.from_ops(
+            cirq.H(b),
+            cirq.CNOT(a, b),
+            cirq.H(b),
+            cirq.CNOT(a, b),
+            cirq.CNOT(b, a),
+            cirq.H(a),
+            cirq.CNOT(a, b),
+            cirq.Z(a)**t, cirq.Z(b)**-t,
+            cirq.CNOT(a, b),
+            cirq.H(a), cirq.Z(b)**v,
+            cirq.CNOT(a, b),
+            cirq.Z(a)**-v, cirq.Z(b)**-v,
         ))
+
+
+def test_optimizes_single_iswap():
+    a, b = cirq.LineQubit.range(2)
+    c = cirq.Circuit.from_ops(cirq.ISWAP(a, b))
+    assert_optimization_not_broken(c)
+    cg.MergeInteractions().optimize_circuit(c)
+    assert len([1 for op in c.all_operations() if len(op.qubits) == 2]) == 2
+    assert all(cg.XmonGate.is_xmon_op(op)
+               for op in c.all_operations())