[Stable] Improve 1q decomposition pass with multiple matching basis (#5431)

qiskit/dagcircuit/dagcircuit.py

@@ -1420,6 +1420,38 @@ def collect_runs(self, namelist):
                     group_list.append(tuple(group))
         return set(group_list)
 
+    # pylint: disable=too-many-boolean-expressions
+
+    def collect_1q_runs(self):
+        """Return a set of non-conditional runs of 1q "op" nodes."""
+        group_list = []
+        # Iterate through the nodes of self in topological order
+        # and form tuples containing sequences of gates
+        # on the same qubit(s).
+        nodes_seen = set()
+        for node in self.topological_op_nodes():
+            if len(node.qargs) == 1 and node.condition is None \
+                    and not node.cargs\
+                    and node not in nodes_seen \
+                    and not node.op.is_parameterized() \
+                    and isinstance(node.op, Gate):
+                group = [node]
+                nodes_seen.add(node)
+                s = self._multi_graph.successors(node._node_id)
+                while len(s) == 1 and \
+                        s[0].type == "op" and \
+                        len(s[0].qargs) == 1 and \
+                        len(s[0].cargs) == 0 and \
+                        s[0].condition is None and \
+                        not s[0].op.is_parameterized() and \
+                        isinstance(node.op, Gate):
+                    group.append(s[0])
+                    nodes_seen.add(s[0])
+                    s = self._multi_graph.successors(s[0]._node_id)
+                if len(group) >= 1:
+                    group_list.append(tuple(group))
+        return set(group_list)
+
     def nodes_on_wire(self, wire, only_ops=False):
         """
         Iterator for nodes that affect a given wire.

qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py

@@ -12,7 +12,6 @@
 
 """Optimize chains of single-qubit gates using Euler 1q decomposer"""
 
-from itertools import groupby
 import logging
 
 import numpy as np
@@ -51,11 +50,11 @@ def __init__(self, basis=None):
         }
         self.basis = None
         if basis:
+            self.basis = []
             basis_set = set(basis)
             for basis_name, gates in self.euler_basis_names.items():
                 if set(gates).issubset(basis_set):
-                    self.basis = basis_name
-                    break
+                    self.basis.append(OneQubitEulerDecomposer(basis_name))
 
     def run(self, dag):
         """Run the Optimize1qGatesDecomposition pass on `dag`.
@@ -69,45 +68,31 @@ def run(self, dag):
         if not self.basis:
             LOG.info("Skipping pass because no basis is set")
             return dag
-        decomposer = OneQubitEulerDecomposer(self.basis)
-        runs = dag.collect_runs(self.euler_basis_names[self.basis])
-        runs = _split_runs_on_parameters(runs)
+        runs = dag.collect_1q_runs()
         for run in runs:
+            # Don't try to optimize a single 1q gate
             if len(run) <= 1:
                 params = run[0].op.params
                 # Remove single identity gates
-                if run[0].op.name in self.euler_basis_names[self.basis] and len(
-                        params) > 0 and np.array_equal(run[0].op.to_matrix(),
-                                                       np.eye(2)):
+                if len(params) > 0 and np.array_equal(run[0].op.to_matrix(),
+                                                      np.eye(2)):
                     dag.remove_op_node(run[0])
-                # Don't try to optimize a single 1q gate
                 continue
+
+            new_circs = []
             q = QuantumRegister(1, "q")
             qc = QuantumCircuit(1)
             for gate in run:
-                qc.append(gate.op, [q[0]], [])
-
+                qc._append(gate.op, [q[0]], [])
             operator = Operator(qc)
-            new_circ = decomposer(operator)
-            new_dag = circuit_to_dag(new_circ)
-            dag.substitute_node_with_dag(run[0], new_dag)
-            # Delete the other nodes in the run
-            for current_node in run[1:]:
-                dag.remove_op_node(current_node)
+            for decomposer in self.basis:
+                new_circs.append(decomposer(operator))
+            if new_circs:
+                new_circ = min(new_circs, key=lambda circ: circ.depth())
+                if qc.depth() > new_circ.depth():
+                    new_dag = circuit_to_dag(new_circ)
+                    dag.substitute_node_with_dag(run[0], new_dag)
+                    # Delete the other nodes in the run
+                    for current_node in run[1:]:
+                        dag.remove_op_node(current_node)
         return dag
-
-
-def _split_runs_on_parameters(runs):
-    """Finds runs containing parameterized gates and splits them into sequential
-    runs excluding the parameterized gates.
-    """
-
-    out = []
-    for run in runs:
-        groups = groupby(run, lambda x: x.op.is_parameterized())
-
-        for group_is_parameterized, gates in groups:
-            if not group_is_parameterized:
-                out.append(list(gates))
-
-    return out

test/python/dagcircuit/test_dagcircuit.py

@@ -25,11 +25,13 @@
 from qiskit.circuit import Measure
 from qiskit.circuit import Reset
 from qiskit.circuit import Gate, Instruction
+from qiskit.circuit import Parameter
 from qiskit.circuit.library.standard_gates.i import IGate
 from qiskit.circuit.library.standard_gates.h import HGate
 from qiskit.circuit.library.standard_gates.x import CXGate
 from qiskit.circuit.library.standard_gates.z import CZGate
 from qiskit.circuit.library.standard_gates.x import XGate
+from qiskit.circuit.library.standard_gates.y import YGate
 from qiskit.circuit.library.standard_gates.u1 import U1Gate
 from qiskit.circuit.barrier import Barrier
 from qiskit.dagcircuit.exceptions import DAGCircuitError
@@ -708,6 +710,112 @@ def test_dag_collect_runs_conditional_in_middle(self):
             self.assertEqual(['h'], [x.name for x in run])
             self.assertEqual([[self.qubit0]], [x.qargs for x in run])
 
+    def test_dag_collect_1q_runs(self):
+        """Test the collect_1q_runs method with 3 different gates."""
+        self.dag.apply_operation_back(U1Gate(3.14), [self.qubit0])
+        self.dag.apply_operation_back(U1Gate(3.14), [self.qubit0])
+        self.dag.apply_operation_back(U1Gate(3.14), [self.qubit0])
+        self.dag.apply_operation_back(CXGate(), [self.qubit2, self.qubit1])
+        self.dag.apply_operation_back(CXGate(), [self.qubit1, self.qubit2])
+        self.dag.apply_operation_back(HGate(), [self.qubit2])
+        collected_runs = self.dag.collect_1q_runs()
+        self.assertEqual(len(collected_runs), 2)
+        for run in collected_runs:
+            if run[0].name == 'h':
+                self.assertEqual(len(run), 1)
+                self.assertEqual(['h'], [x.name for x in run])
+                self.assertEqual([[self.qubit2]], [x.qargs for x in run])
+            elif run[0].name == 'u1':
+                self.assertEqual(len(run), 3)
+                self.assertEqual(['u1'] * 3, [x.name for x in run])
+                self.assertEqual(
+                    [[self.qubit0], [self.qubit0], [self.qubit0]],
+                    [x.qargs for x in run])
+            else:
+                self.fail('Unknown run encountered')
+
+    def test_dag_collect_1q_runs_start_with_conditional(self):
+        """Test collect 1q runs with a conditional at the start of the run."""
+        h_gate = HGate()
+        h_gate.condition = self.condition
+        self.dag.apply_operation_back(
+            h_gate, [self.qubit0])
+        self.dag.apply_operation_back(HGate(), [self.qubit0])
+        self.dag.apply_operation_back(HGate(), [self.qubit0])
+        collected_runs = self.dag.collect_1q_runs()
+        self.assertEqual(len(collected_runs), 1)
+        run = collected_runs.pop()
+        self.assertEqual(len(run), 2)
+        self.assertEqual(['h', 'h'], [x.name for x in run])
+        self.assertEqual([[self.qubit0], [self.qubit0]],
+                         [x.qargs for x in run])
+
+    def test_dag_collect_1q_runs_conditional_in_middle(self):
+        """Test collect_1q_runs with a conditional in the middle of a run."""
+        h_gate = HGate()
+        h_gate.condition = self.condition
+        self.dag.apply_operation_back(HGate(), [self.qubit0])
+        self.dag.apply_operation_back(
+            h_gate, [self.qubit0])
+        self.dag.apply_operation_back(HGate(), [self.qubit0])
+        collected_runs = self.dag.collect_1q_runs()
+        # Should return 2 single h gate runs (1 before condition, 1 after)
+        self.assertEqual(len(collected_runs), 2)
+        for run in collected_runs:
+            self.assertEqual(len(run), 1)
+            self.assertEqual(['h'], [x.name for x in run])
+            self.assertEqual([[self.qubit0]], [x.qargs for x in run])
+
+    def test_dag_collect_1q_runs_with_parameterized_gate(self):
+        """Test collect 1q splits on parameterized gates."""
+        theta = Parameter('theta')
+        self.dag.apply_operation_back(HGate(), [self.qubit0])
+        self.dag.apply_operation_back(HGate(), [self.qubit0])
+        self.dag.apply_operation_back(U1Gate(theta), [self.qubit0])
+        self.dag.apply_operation_back(XGate(), [self.qubit0])
+        self.dag.apply_operation_back(XGate(), [self.qubit0])
+        collected_runs = self.dag.collect_1q_runs()
+        self.assertEqual(len(collected_runs), 2)
+        run_gates = [[x.name for x in run] for run in collected_runs]
+        self.assertIn(['h', 'h'], run_gates)
+        self.assertIn(['x', 'x'], run_gates)
+        self.assertNotIn('u1', [x.name for run in collected_runs for x in run])
+
+    def test_dag_collect_1q_runs_with_cx_in_middle(self):
+        """Test collect_1q_runs_with a cx in the middle of the run."""
+        self.dag.apply_operation_back(HGate(), [self.qubit0])
+        self.dag.apply_operation_back(HGate(), [self.qubit0])
+        self.dag.apply_operation_back(U1Gate(3.14), [self.qubit0])
+        self.dag.apply_operation_back(U1Gate(3.14), [self.qubit1])
+        self.dag.apply_operation_back(U1Gate(3.14), [self.qubit1])
+        self.dag.apply_operation_back(HGate(), [self.qubit1])
+        self.dag.apply_operation_back(CXGate(), [self.qubit0, self.qubit1])
+        self.dag.apply_operation_back(YGate(), [self.qubit0])
+        self.dag.apply_operation_back(YGate(), [self.qubit0])
+        self.dag.apply_operation_back(XGate(), [self.qubit1])
+        self.dag.apply_operation_back(XGate(), [self.qubit1])
+        collected_runs = self.dag.collect_1q_runs()
+        self.assertEqual(len(collected_runs), 4)
+        for run in collected_runs:
+            if run[0].name == 'h':
+                self.assertEqual(len(run), 3)
+                self.assertEqual(['h', 'h', 'u1'], [x.name for x in run])
+                self.assertEqual([[self.qubit0]] * 3, [x.qargs for x in run])
+            elif run[0].name == 'u1':
+                self.assertEqual(len(run), 3)
+                self.assertEqual(['u1', 'u1', 'h'], [x.name for x in run])
+                self.assertEqual([[self.qubit1]] * 3, [x.qargs for x in run])
+            elif run[0].name == 'x':
+                self.assertEqual(len(run), 2)
+                self.assertEqual(['x', 'x'], [x.name for x in run])
+                self.assertEqual([[self.qubit1]] * 2, [x.qargs for x in run])
+            elif run[0].name == 'y':
+                self.assertEqual(len(run), 2)
+                self.assertEqual(['y', 'y'], [x.name for x in run])
+                self.assertEqual([[self.qubit0]] * 2, [x.qargs for x in run])
+            else:
+                self.fail("Unknown run encountered")
+
 
 class TestDagLayers(QiskitTestCase):
     """Test finding layers on the dag"""

test/python/transpiler/test_optimize_1q_decomposition.py

@@ -19,6 +19,7 @@
 
 from qiskit.circuit import QuantumRegister, QuantumCircuit, ClassicalRegister
 from qiskit.circuit.library.standard_gates import U3Gate
+from qiskit.circuit.random import random_circuit
 from qiskit.transpiler import PassManager
 from qiskit.transpiler.passes import Optimize1qGatesDecomposition
 from qiskit.transpiler.passes import BasisTranslator
@@ -316,6 +317,33 @@ def test_identity_u1x(self):
         result = passmanager.run(circuit)
         self.assertEqual([], result.data)
 
+    def test_overcomplete_basis(self):
+        """Test optimization with an overcomplete basis."""
+        circuit = random_circuit(3, 3, seed=42)
+        basis = ['rz', 'rxx', 'rx', 'ry', 'p', 'sx', 'u', 'cx']
+        passmanager = PassManager()
+        passmanager.append(BasisTranslator(sel, basis))
+        basis_translated = passmanager.run(circuit)
+        passmanager = PassManager()
+        passmanager.append(Optimize1qGatesDecomposition(basis))
+        result_full = passmanager.run(basis_translated)
+        self.assertTrue(Operator(circuit).equiv(Operator(result_full)))
+        self.assertGreater(basis_translated.depth(), result_full.depth())
+
+    def test_euler_decomposition_worse(self):
+        """Ensure we don't decompose to a deeper circuit."""
+        circuit = QuantumCircuit(1)
+        circuit.rx(-np.pi / 2, 0)
+        circuit.rz(-np.pi / 2, 0)
+        basis = ['rx', 'rz']
+        passmanager = PassManager()
+        passmanager.append(BasisTranslator(sel, basis))
+        passmanager.append(Optimize1qGatesDecomposition(basis))
+        result = passmanager.run(circuit)
+        # decomposition of circuit will result in 3 gates instead of 2
+        # assert optimization pass doesn't use it.
+        self.assertEqual(result, circuit)
+
 
 if __name__ == '__main__':
     unittest.main()