From 8d093850ccc41236b1b7ad3ba41aa218e3aee078 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Thu, 10 Dec 2020 23:39:30 -0500 Subject: [PATCH 1/5] Improve 1q decomposition pass with multiple matching basis (#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 * Fix lint * Use qc._append instead of qc.append Conflicts/Changes: For backport this makes 1 key change, the collect_1q_runs method is rewritten to be pure python instead of leveraging the retworkx collect_runs() method which was added in a newer version of retworkx than the minimum allowed version. This also partially pulls in the changes made in #5570, but only some were backportable since the others relied on attributes that don't exist in gate objects in 0.16.x. (cherry picked from commit 5a1768f8a0522513ca6ec2a93b36752f103b18ba) --- qiskit/dagcircuit/dagcircuit.py | 31 +++++ .../optimization/optimize_1q_decomposition.py | 53 +++------ test/python/dagcircuit/test_dagcircuit.py | 108 ++++++++++++++++++ .../test_optimize_1q_decomposition.py | 28 +++++ 4 files changed, 186 insertions(+), 34 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 8f1015094f3a..62d1e331ad7a 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -1420,6 +1420,37 @@ def collect_runs(self, namelist): group_list.append(tuple(group)) return set(group_list) + 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). + topo_ops = list(self.topological_op_nodes()) + nodes_seen = dict(zip(topo_ops, [False] * len(topo_ops))) + for node in topo_ops: + if len(node.qargs) == 1 and node.condition is None \ + and not node.cargs \ + and not node.op.is_parameterized() \ + and isinstance(node.op, Gate) \ + and not nodes_seen[node]: + group = [node] + nodes_seen[node] = True + 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[s[0]] = True + 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. diff --git a/qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py b/qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py index 23c028106758..ce9e82090bbc 100644 --- a/qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py +++ b/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 diff --git a/test/python/dagcircuit/test_dagcircuit.py b/test/python/dagcircuit/test_dagcircuit.py index b746ec3b7cbd..83f85ec0efcf 100644 --- a/test/python/dagcircuit/test_dagcircuit.py +++ b/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""" diff --git a/test/python/transpiler/test_optimize_1q_decomposition.py b/test/python/transpiler/test_optimize_1q_decomposition.py index 9708f0b9ca99..620be85b0358 100644 --- a/test/python/transpiler/test_optimize_1q_decomposition.py +++ b/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() From 1cc5491645e0f83056f0cd5ea0f9a4a14e18018e Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Tue, 19 Jan 2021 14:23:35 -0500 Subject: [PATCH 2/5] Fix lint --- qiskit/dagcircuit/dagcircuit.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 62d1e331ad7a..239e1aa32c90 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -10,6 +10,8 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. +# pylint: disable=too-many-boolean-expressions + """ Object to represent a quantum circuit as a directed acyclic graph (DAG). From b202cb4db48b26177602d91a40cd3a3ffd4c52b6 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Tue, 19 Jan 2021 17:19:46 -0500 Subject: [PATCH 3/5] Simplify collect_1q_runs --- qiskit/dagcircuit/dagcircuit.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 239e1aa32c90..53e6c7a71839 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -1428,16 +1428,15 @@ def collect_1q_runs(self): # Iterate through the nodes of self in topological order # and form tuples containing sequences of gates # on the same qubit(s). - topo_ops = list(self.topological_op_nodes()) - nodes_seen = dict(zip(topo_ops, [False] * len(topo_ops))) - for node in topo_ops: + 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 not node.op.is_parameterized() \ and isinstance(node.op, Gate) \ - and not nodes_seen[node]: + and node not in nodes_seen: group = [node] - nodes_seen[node] = True + nodes_seen.add(node) s = self._multi_graph.successors(node._node_id) while len(s) == 1 and \ s[0].type == "op" and \ @@ -1447,7 +1446,7 @@ def collect_1q_runs(self): not s[0].op.is_parameterized() and \ isinstance(node.op, Gate): group.append(s[0]) - nodes_seen[s[0]] = True + nodes_seen.add(s[0]) s = self._multi_graph.successors(s[0]._node_id) if len(group) >= 1: group_list.append(tuple(group)) From aafb8b65ae6da5b503d6b4137b202c1a02832865 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Tue, 19 Jan 2021 17:21:47 -0500 Subject: [PATCH 4/5] Move pylint disable to just collect_1q_runs --- qiskit/dagcircuit/dagcircuit.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 53e6c7a71839..39c4c6317527 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -10,8 +10,6 @@ # copyright notice, and modified files need to carry a notice indicating # that they have been altered from the originals. -# pylint: disable=too-many-boolean-expressions - """ Object to represent a quantum circuit as a directed acyclic graph (DAG). @@ -1422,6 +1420,8 @@ 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 = [] From 90f9bce5b7ee930e1fbf88289f69a994ce6dfdf8 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 20 Jan 2021 17:42:13 -0500 Subject: [PATCH 5/5] Reorder 1q node checks for performance --- qiskit/dagcircuit/dagcircuit.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qiskit/dagcircuit/dagcircuit.py b/qiskit/dagcircuit/dagcircuit.py index 39c4c6317527..f560411a7eba 100644 --- a/qiskit/dagcircuit/dagcircuit.py +++ b/qiskit/dagcircuit/dagcircuit.py @@ -1431,10 +1431,10 @@ def collect_1q_runs(self): 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 not node.cargs\ + and node not in nodes_seen \ and not node.op.is_parameterized() \ - and isinstance(node.op, Gate) \ - and node not in nodes_seen: + and isinstance(node.op, Gate): group = [node] nodes_seen.add(node) s = self._multi_graph.successors(node._node_id)