Improve 1q decomposition pass with multiple matching basis (Qiskit#5431)

* Improve 1q decomposition pass with multiple matching basis

This commit makes an improvement to the Optimize1qGatesDecomposition
pass to improve the output from it in two scenarios. The first scenario
is if there is an over complete basis where there are multiple choices
for decomposition basis. Previously in such scenarios only the first
matching subset would be used. This improves that by finding runs in all
matching decomposition basis to simplify any runs in those gate sets.
The second improvement made here is that the decomposition is only used
if it results in a decrease in depth. There were scenarios where the
general decomposition of a run would result in more gates than the
input to the OneQubitEulerDecomposer (typically 3 gates vs 2 input). In
those cases we shouldn't use the decomposition and instead rely on the
original run because we're adding gates which is the opposit of the
expected behavior.

* Add DAGCircuit method to get 1q op node runs

This commit adds a new DAGCircuit method, collect_1q_runs, which behaves
like collect_runs(), but instead of collecting runs with gates on a name
filter it looks at the number of qubits and returns runs of any op node
that operate on a single qubit. This will be used by the optimize 1q
decomposition pass to collect arbitrary single qubit gate runs and
decompose them over the basis sets.

* Switch to use all 1q runs instead of those just matching basis

* Use retworkx.collect_runs for collect_1q_runs

* Move parameterized gate split to collect_1q_runs

* Reverse loops so basis is the inner loop and runs the outer

* Ensure collect_1q_runs doesn't collect measurements

* Only create temp circuit once per run

* Move lone identity optimization into outer loop

* Update qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py

Co-authored-by: Kevin Krsulich <kevin@krsulich.net>

* Fix lint

* Use qc._append instead of qc.append

Co-authored-by: Kevin Krsulich <kevin@krsulich.net>
Co-authored-by: Kevin Krsulich <kevin.krsulich@ibm.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

qiskit/dagcircuit/dagcircuit.py

@@ -1377,6 +1377,17 @@ def filter_fn(node):
         group_list = rx.collect_runs(self._multi_graph, filter_fn)
         return set(tuple(x) for x in group_list)
 
+    def collect_1q_runs(self):
+        """Return a set of non-conditional runs of 1q "op" nodes."""
+
+        def filter_fn(node):
+            return node.type == 'op' and len(node.qargs) == 1 \
+                and len(node.cargs) == 0 and node.condition is None \
+                and not node.op.is_parameterized() \
+
+        group_list = rx.collect_runs(self._multi_graph, filter_fn)
+        return set(tuple(x) for x in 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()