From fd85047d3ddca53d4e55c793c71b77827b71774a Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Thu, 1 Dec 2022 14:39:22 -0500 Subject: [PATCH] Enable simplifying 1q runs and 2q blocks in transpile() without target (#9222) * Enable simplifying 1q runs and 2q blocks in transpile() without target This commit fixes an issue when transpile() was called with optimization enabled (optimization levels 1, 2, and 3) and no target (or basis gates) specified then 1q run or 2q block optimization was run. This would result in long series of gates that could be simplified being left in the output circuit. To address this, for 1q optimization the Optimize1qGatesDecomposition pass (which is run at all 3 optimization levels) is updated so that if no target is specified we just try all decomposers as the default heuristic for the best output is the shortest sequence length (in the absense of error rates from the target) and if any output gate is valid that will either remove the 1q sequence if it's an identity sequence, or likely be a single gate. As no basis is specified this behavior is fine since the calculations are quick and any output basis will match the constraints the user provided the transpiler. For optimization level 3 with it's 2q blcok optimization with the UnitarySynthesis pass it is a bit more involved though. The cost of doing the unitary synthesis is higher, the number of possible decompositions is larger, and we don't have a good heuristic measure of which would perform best without a target specified and it's not feasible to just try all supported basis by the synthesis module. This means for a non-identity 2 qubit block the output will be a UnitaryGate (which without a target specified is a valid output). However, to address the case when an identity block is present in the circuit this can be removed with very little overhead. To accomplish this the ConsolidateBlocks pass is updated to check if an identified 2 qubit block is equal the identity matrix and if so will remove that block from the circuit. Fixes #9217 * Fix docs build * Fix handling of 1q decomposer logic with no basis gates This commit fixes an oversight in the previous commit for the 1q decomposer pass when no basis gates are specified. Previously we were setting the basis list to be the set of all the gates in the supported euler basis for the decomposer. This had the unintended side effect of breaking the heuristic for the decomposer as it would treat any 1q gate outside of the supported euler basis as needing to be translated. This was not the desired behavior as any gate is valid. This fixes the logic to just treat no basis explicitly as everything being valid output and weight the heuristic error only. * Remove debug prints * Remove unnecessary conditional 1q identity matrix creation * Split out release notes for 1q and 2q cases Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .../passes/optimization/consolidate_blocks.py | 18 ++++++++-- .../optimization/optimize_1q_decomposition.py | 35 ++++++++++++------- qiskit/visualization/timeline/core.py | 5 ++- ...lification-no-target-62cd8614044a0fe9.yaml | 24 +++++++++++++ test/python/circuit/test_scheduled_circuit.py | 7 +++- test/python/compiler/test_transpiler.py | 21 +++++++++++ .../transpiler/test_consolidate_blocks.py | 21 +++++++++++ .../test_optimize_1q_decomposition.py | 10 ++++++ 8 files changed, 124 insertions(+), 17 deletions(-) create mode 100644 releasenotes/notes/fix-identity-simplification-no-target-62cd8614044a0fe9.yaml diff --git a/qiskit/transpiler/passes/optimization/consolidate_blocks.py b/qiskit/transpiler/passes/optimization/consolidate_blocks.py index 4168ed17f0e1..99df3a4d8ee7 100644 --- a/qiskit/transpiler/passes/optimization/consolidate_blocks.py +++ b/qiskit/transpiler/passes/optimization/consolidate_blocks.py @@ -12,6 +12,8 @@ """Replace each block of consecutive gates by a single Unitary node.""" +import numpy as np + from qiskit.circuit.classicalregister import ClassicalRegister from qiskit.circuit.quantumregister import QuantumRegister from qiskit.circuit.quantumcircuit import QuantumCircuit @@ -113,9 +115,17 @@ def run(self, dag): or ((self.basis_gates is not None) and outside_basis) or ((self.target is not None) and outside_basis) ): - dag.replace_block_with_op(block, unitary, block_index_map, cycle_check=False) + identity = np.eye(2**unitary.num_qubits) + if np.allclose(identity, unitary.to_matrix()): + for node in block: + dag.remove_op_node(node) + else: + dag.replace_block_with_op( + block, unitary, block_index_map, cycle_check=False + ) # If 1q runs are collected before consolidate those too runs = self.property_set["run_list"] or [] + identity_1q = np.eye(2) for run in runs: if any(gate in all_block_gates for gate in run): continue @@ -134,7 +144,11 @@ def run(self, dag): if already_in_block: continue unitary = UnitaryGate(operator) - dag.replace_block_with_op(run, unitary, {qubit: 0}, cycle_check=False) + if np.allclose(identity_1q, unitary.to_matrix()): + for node in run: + dag.remove_op_node(node) + else: + dag.replace_block_with_op(run, unitary, {qubit: 0}, cycle_check=False) # Clear collected blocks and runs as they are no longer valid after consolidation if "run_list" in self.property_set: del self.property_set["run_list"] diff --git a/qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py b/qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py index 2a529540bb73..51b21aca7e99 100644 --- a/qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py +++ b/qiskit/transpiler/passes/optimization/optimize_1q_decomposition.py @@ -55,6 +55,9 @@ def __init__(self, basis=None, target=None): if basis: self._global_decomposers = _possible_decomposers(set(basis)) + elif target is None: + self._global_decomposers = _possible_decomposers(None) + self._basis_gates = None def _resynthesize_run(self, run, qubit=None): """ @@ -97,10 +100,14 @@ def _substitution_checks(self, dag, old_run, new_circ, basis, qubit): # does this run have uncalibrated gates? uncalibrated_p = not has_cals_p or any(not dag.has_calibration_for(g) for g in old_run) # does this run have gates not in the image of ._decomposers _and_ uncalibrated? - uncalibrated_and_not_basis_p = any( - g.name not in basis and (not has_cals_p or not dag.has_calibration_for(g)) - for g in old_run - ) + if basis is not None: + uncalibrated_and_not_basis_p = any( + g.name not in basis and (not has_cals_p or not dag.has_calibration_for(g)) + for g in old_run + ) + else: + # If no basis is specified then we're always in the basis + uncalibrated_and_not_basis_p = False # if we're outside of the basis set, we're obligated to logically decompose. # if we're outside of the set of gates for which we have physical definitions, @@ -124,10 +131,6 @@ def run(self, dag): Returns: DAGCircuit: the optimized DAG. """ - if self._basis_gates is None and self._target is None: - logger.info("Skipping pass because no basis or target is set") - return dag - runs = dag.collect_1q_runs() qubit_indices = {bit: index for index, bit in enumerate(dag.qubits)} for run in runs: @@ -151,11 +154,17 @@ def run(self, dag): def _possible_decomposers(basis_set): decomposers = [] - euler_basis_gates = one_qubit_decompose.ONE_QUBIT_EULER_BASIS_GATES - for euler_basis_name, gates in euler_basis_gates.items(): - if set(gates).issubset(basis_set): - decomposer = one_qubit_decompose.OneQubitEulerDecomposer(euler_basis_name) - decomposers.append(decomposer) + if basis_set is None: + decomposers = [ + one_qubit_decompose.OneQubitEulerDecomposer(basis) + for basis in one_qubit_decompose.ONE_QUBIT_EULER_BASIS_GATES + ] + else: + euler_basis_gates = one_qubit_decompose.ONE_QUBIT_EULER_BASIS_GATES + for euler_basis_name, gates in euler_basis_gates.items(): + if set(gates).issubset(basis_set): + decomposer = one_qubit_decompose.OneQubitEulerDecomposer(euler_basis_name) + decomposers.append(decomposer) return decomposers diff --git a/qiskit/visualization/timeline/core.py b/qiskit/visualization/timeline/core.py index 5cecfcdb9a65..51610655fde5 100644 --- a/qiskit/visualization/timeline/core.py +++ b/qiskit/visualization/timeline/core.py @@ -162,7 +162,10 @@ def load_program(self, program: circuit.QuantumCircuit): try: program = transpile( - program, scheduling_method="alap", instruction_durations=InstructionDurations() + program, + scheduling_method="alap", + instruction_durations=InstructionDurations(), + optimization_level=0, ) except TranspilerError as ex: raise VisualizationError( diff --git a/releasenotes/notes/fix-identity-simplification-no-target-62cd8614044a0fe9.yaml b/releasenotes/notes/fix-identity-simplification-no-target-62cd8614044a0fe9.yaml new file mode 100644 index 000000000000..5c1326d3942a --- /dev/null +++ b/releasenotes/notes/fix-identity-simplification-no-target-62cd8614044a0fe9.yaml @@ -0,0 +1,24 @@ +--- +fixes: + - | + Fixed an issue with the :func:`~.transpile` function when run with + ``optimization_level`` set to ``1``, ``2``, or ``3`` and no + ``backend``, ``basis_gates``, or ``target`` argument specified. If + the input circuit had runs of single qubit gates which could be simplified + the output circuit would not be as optimized as possible as those runs + of single qubit gates would not have been removed. This could have been + corrected previously by specifying either the ``backend``, ``basis_gates``, + or ``target`` arguments on the :func:`~.transpile` call, but now the output + will be as simplified as it can be without knowing the target gates allowed. + Fixed `#9217 `__ + - | + Fixed an issue with the :func:`~.transpile` function when run with + ``optimization_level=3`` and no ``backend``, ``basis_gates``, or ``target`` + argument specified. If the input circuit contained any 2 qubit blocks which + were equivalent to an identity matrix the output circuit would not be as + optimized as possible and and would still contain that identity block. + This could have been corrected previously by specifying either the + ``backend``, ``basis_gates``, or ``target`` arguments on the + :func:`~.transpile` call, but now the output will be as simplified as it + can be without knowing the target gates allowed. + Fixed `#9217 `__ diff --git a/test/python/circuit/test_scheduled_circuit.py b/test/python/circuit/test_scheduled_circuit.py index ad8f66e0d27d..872a79e3e0f6 100644 --- a/test/python/circuit/test_scheduled_circuit.py +++ b/test/python/circuit/test_scheduled_circuit.py @@ -154,7 +154,10 @@ def test_transpile_delay_circuit_without_backend(self): qc.delay(500, 1) qc.cx(0, 1) scheduled = transpile( - qc, scheduling_method="alap", instruction_durations=[("h", 0, 200), ("cx", [0, 1], 700)] + qc, + scheduling_method="alap", + basis_gates=["h", "cx"], + instruction_durations=[("h", 0, 200), ("cx", [0, 1], 700)], ) self.assertEqual(scheduled.duration, 1200) @@ -259,6 +262,7 @@ def test_per_qubit_durations(self): sc = transpile( qc, scheduling_method="alap", + basis_gates=["h", "cx"], instruction_durations=[("h", None, 200), ("cx", [0, 1], 700)], ) self.assertEqual(sc.qubit_start_time(0), 300) @@ -274,6 +278,7 @@ def test_per_qubit_durations(self): sc = transpile( qc, scheduling_method="alap", + basis_gates=["h", "cx", "measure"], instruction_durations=[("h", None, 200), ("cx", [0, 1], 700), ("measure", None, 1000)], ) q = sc.qubits diff --git a/test/python/compiler/test_transpiler.py b/test/python/compiler/test_transpiler.py index 4964c9c723c6..0c984b3875d7 100644 --- a/test/python/compiler/test_transpiler.py +++ b/test/python/compiler/test_transpiler.py @@ -1542,6 +1542,27 @@ def _visit_block(circuit, qubit_mapping=None): qubit_mapping={qubit: index for index, qubit in enumerate(transpiled.qubits)}, ) + @data(1, 2, 3) + def test_transpile_identity_circuit_no_target(self, opt_level): + """Test circuit equivalent to identity is optimized away for all optimization levels >0. + + Reproduce taken from https://github.com/Qiskit/qiskit-terra/issues/9217 + """ + qr1 = QuantumRegister(3, "state") + qr2 = QuantumRegister(2, "ancilla") + cr = ClassicalRegister(2, "c") + qc = QuantumCircuit(qr1, qr2, cr) + qc.h(qr1[0]) + qc.cx(qr1[0], qr1[1]) + qc.cx(qr1[1], qr1[2]) + qc.cx(qr1[1], qr1[2]) + qc.cx(qr1[0], qr1[1]) + qc.h(qr1[0]) + + empty_qc = QuantumCircuit(qr1, qr2, cr) + result = transpile(qc, optimization_level=opt_level) + self.assertEqual(empty_qc, result) + @ddt class TestPostTranspileIntegration(QiskitTestCase): diff --git a/test/python/transpiler/test_consolidate_blocks.py b/test/python/transpiler/test_consolidate_blocks.py index bc51927f98f3..3332026e436b 100644 --- a/test/python/transpiler/test_consolidate_blocks.py +++ b/test/python/transpiler/test_consolidate_blocks.py @@ -407,6 +407,27 @@ def test_single_gate_block_outside_target_with_matching_basis_gates(self): expected.swap(0, 1) self.assertEqual(expected, pass_manager.run(qc)) + def test_identity_unitary_is_removed(self): + """Test that a 2q identity unitary is removed without a basis.""" + qc = QuantumCircuit(5) + qc.h(0) + qc.cx(0, 1) + qc.cx(0, 1) + qc.h(0) + + pm = PassManager([Collect2qBlocks(), ConsolidateBlocks()]) + self.assertEqual(QuantumCircuit(5), pm.run(qc)) + + def test_identity_1q_unitary_is_removed(self): + """Test that a 1q identity unitary is removed without a basis.""" + qc = QuantumCircuit(5) + qc.h(0) + qc.h(0) + qc.h(0) + qc.h(0) + pm = PassManager([Collect2qBlocks(), Collect1qRuns(), ConsolidateBlocks()]) + self.assertEqual(QuantumCircuit(5), pm.run(qc)) + if __name__ == "__main__": unittest.main() diff --git a/test/python/transpiler/test_optimize_1q_decomposition.py b/test/python/transpiler/test_optimize_1q_decomposition.py index 3e8b0fc0909c..ced8a6d63350 100644 --- a/test/python/transpiler/test_optimize_1q_decomposition.py +++ b/test/python/transpiler/test_optimize_1q_decomposition.py @@ -135,6 +135,16 @@ def test_optimize_identity_target(self, target): result = passmanager.run(circuit) self.assertEqual(expected, result) + def test_optimize_away_idenity_no_target(self): + """Test identity run is removed for no target specified.""" + circuit = QuantumCircuit(1) + circuit.h(0) + circuit.h(0) + passmanager = PassManager() + passmanager.append(Optimize1qGatesDecomposition()) + result = passmanager.run(circuit) + self.assertEqual(QuantumCircuit(1), result) + def test_optimize_error_over_target_1(self): """XZX is re-written as ZXZ, which is cheaper according to target.""" qr = QuantumRegister(1, "qr")