From 10480857223ac024d3682165f6ecaf71e91abe9c Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 7 Aug 2024 20:16:33 -0400 Subject: [PATCH] Remove init peephole optimization discrete basis check (#12898) * Fix target handling in discrete basis check In #12727 a check was added to the default init stage's construction to avoid running 2q gate consolidation in the presence of targets with only discrete gates. However the way the target was being used in this check was incorrect. The name for an instruction in the target should be used as its identifier and then if we need the object representation that should query the target for that object based on the name. However the check was doing this backwards getting a list of operation objects and then using the name contained in the object. This will cause issues for instructions that use custom names such as when there are tuned variants or a custom gate instance with a unique name. While there is some question over whether we need this check as we will run the consolidate 2q blocks pass as part of the optimization stage which will have the same effect, this opts to just fix the target usage for it to minimize the diff. Also while not the explicit goal of this check it is protecting against some bugs in the consolidate blocks pass that occur when custom gates are used. So for the short term this check is retained, but in the future when these bugs in consolidate blocks are fixed we can revisit whether we want to remove the conditional logic. * Remove check and fix ConsolidateBlocks bug This commit pivots this PR branch to just remove the additional logic around skipping the optimization passes for discrete basis sets. The value the check was actually providing was not around a discrete basis set target and instead was to workaround a bug in the consolidate blocks pass. If a discrete basis set target was used this would still fail because we will unconditionally call `ConsolidateBlocks` during the optimization stage. This commit opts to just remove the extra complexity of the conditional execution of the peephole optimization passes and instead just fix the underlying bug in `ConsolidateBlocks` and remove the check. (cherry picked from commit 70c2f7813c27c557a1681d4c6b461e2f07ebca61) --- .../passes/optimization/consolidate_blocks.py | 8 ++- .../preset_passmanagers/builtin_plugins.py | 64 +++---------------- ...ustom-gate-no-target-e2d1e0b0ee7ace11.yaml | 9 +++ test/python/compiler/test_transpiler.py | 37 ----------- .../transpiler/test_consolidate_blocks.py | 19 +++++- 5 files changed, 44 insertions(+), 93 deletions(-) create mode 100644 releasenotes/notes/fix-consolidate-blocks-custom-gate-no-target-e2d1e0b0ee7ace11.yaml diff --git a/qiskit/transpiler/passes/optimization/consolidate_blocks.py b/qiskit/transpiler/passes/optimization/consolidate_blocks.py index 72a08efe0f7d..12f7285af9dc 100644 --- a/qiskit/transpiler/passes/optimization/consolidate_blocks.py +++ b/qiskit/transpiler/passes/optimization/consolidate_blocks.py @@ -28,6 +28,7 @@ from qiskit.transpiler.passes.synthesis import unitary_synthesis from qiskit.circuit.controlflow import CONTROL_FLOW_OP_NAMES from qiskit._accelerate.convert_2q_block_matrix import blocks_to_matrix +from qiskit.exceptions import QiskitError from .collect_1q_runs import Collect1qRuns from .collect_2q_blocks import Collect2qBlocks @@ -125,7 +126,12 @@ def run(self, dag): qc.append(nd.op, [q[block_index_map[i]] for i in nd.qargs]) unitary = UnitaryGate(Operator(qc), check_input=False) else: - matrix = blocks_to_matrix(block, block_index_map) + try: + matrix = blocks_to_matrix(block, block_index_map) + except QiskitError: + # If building a matrix for the block fails we should not consolidate it + # because there is nothing we can do with it. + continue unitary = UnitaryGate(matrix, check_input=False) max_2q_depth = 20 # If depth > 20, there will be 1q gates to consolidate. diff --git a/qiskit/transpiler/preset_passmanagers/builtin_plugins.py b/qiskit/transpiler/preset_passmanagers/builtin_plugins.py index d7e6a3b2c174..7e3e2611546f 100644 --- a/qiskit/transpiler/preset_passmanagers/builtin_plugins.py +++ b/qiskit/transpiler/preset_passmanagers/builtin_plugins.py @@ -14,7 +14,6 @@ import os -from qiskit.circuit import Instruction from qiskit.transpiler.passes.optimization.split_2q_unitaries import Split2QUnitaries from qiskit.transpiler.passmanager import PassManager from qiskit.transpiler.exceptions import TranspilerError @@ -66,7 +65,6 @@ CYGate, SXGate, SXdgGate, - get_standard_gate_name_mapping, ) from qiskit.utils.parallel import CPU_COUNT from qiskit import user_config @@ -173,58 +171,16 @@ def pass_manager(self, pass_manager_config, optimization_level=None) -> PassMana ) ) init.append(CommutativeCancellation()) - # skip peephole optimization before routing if target basis gate set is discrete, - # i.e. only consists of Cliffords that an user might want to keep - # use rz, sx, x, cx as basis, rely on physical optimziation to fix everything later one - stdgates = get_standard_gate_name_mapping() - - def _is_one_op_non_discrete(ops): - """Checks if one operation in `ops` is not discrete, i.e. is parameterizable - Args: - ops (List(Operation)): list of operations to check - Returns - True if at least one operation in `ops` is not discrete, False otherwise - """ - found_one_continuous_gate = False - for op in ops: - if isinstance(op, str): - if op in _discrete_skipped_ops: - continue - op = stdgates.get(op, None) - - if op is not None and op.name in _discrete_skipped_ops: - continue - - if op is None or not isinstance(op, Instruction): - return False - - if len(op.params) > 0: - found_one_continuous_gate = True - return found_one_continuous_gate - - target = pass_manager_config.target - basis = pass_manager_config.basis_gates - # consolidate gates before routing if the user did not specify a discrete basis gate, i.e. - # * no target or basis gate set has been specified - # * target has been specified, and we have one non-discrete gate in the target's spec - # * basis gates have been specified, and we have one non-discrete gate in that set - do_consolidate_blocks_init = target is None and basis is None - do_consolidate_blocks_init |= target is not None and _is_one_op_non_discrete( - target.operations - ) - do_consolidate_blocks_init |= basis is not None and _is_one_op_non_discrete(basis) - - if do_consolidate_blocks_init: - init.append(Collect2qBlocks()) - init.append(ConsolidateBlocks()) - # If approximation degree is None that indicates a request to approximate up to the - # error rates in the target. However, in the init stage we don't yet know the target - # qubits being used to figure out the fidelity so just use the default fidelity parameter - # in this case. - if pass_manager_config.approximation_degree is not None: - init.append(Split2QUnitaries(pass_manager_config.approximation_degree)) - else: - init.append(Split2QUnitaries()) + init.append(Collect2qBlocks()) + init.append(ConsolidateBlocks()) + # If approximation degree is None that indicates a request to approximate up to the + # error rates in the target. However, in the init stage we don't yet know the target + # qubits being used to figure out the fidelity so just use the default fidelity parameter + # in this case. + if pass_manager_config.approximation_degree is not None: + init.append(Split2QUnitaries(pass_manager_config.approximation_degree)) + else: + init.append(Split2QUnitaries()) else: raise TranspilerError(f"Invalid optimization level {optimization_level}") return init diff --git a/releasenotes/notes/fix-consolidate-blocks-custom-gate-no-target-e2d1e0b0ee7ace11.yaml b/releasenotes/notes/fix-consolidate-blocks-custom-gate-no-target-e2d1e0b0ee7ace11.yaml new file mode 100644 index 000000000000..e4cf03778e3a --- /dev/null +++ b/releasenotes/notes/fix-consolidate-blocks-custom-gate-no-target-e2d1e0b0ee7ace11.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixed a bug in the :class:`.ConsolidateBlocks` transpiler pass, when the + input circuit contains a custom opaque gate and neither the + ``basis_gates`` or ``target`` options are set the pass would raise a + ``QiskitError`` and fail. This has been corrected so that the in these + situations the transpiler pass will not consolidate the block identified + containing a custom gate instead of failing. diff --git a/test/python/compiler/test_transpiler.py b/test/python/compiler/test_transpiler.py index 47557907966c..a29bfc21c8bb 100644 --- a/test/python/compiler/test_transpiler.py +++ b/test/python/compiler/test_transpiler.py @@ -84,7 +84,6 @@ from qiskit.transpiler import CouplingMap, Layout, PassManager, TransformationPass from qiskit.transpiler.exceptions import TranspilerError, CircuitTooWideForTarget from qiskit.transpiler.passes import BarrierBeforeFinalMeasurements, GateDirection, VF2PostLayout -from qiskit.transpiler.passes.optimization.split_2q_unitaries import Split2QUnitaries from qiskit.transpiler.passmanager_config import PassManagerConfig from qiskit.transpiler.preset_passmanagers import generate_preset_pass_manager, level_0_pass_manager @@ -864,42 +863,6 @@ def test_do_not_run_gatedirection_with_symmetric_cm(self): transpile(circ, coupling_map=coupling_map, initial_layout=layout) self.assertFalse(mock_pass.called) - def tests_conditional_run_split_2q_unitaries(self): - """Tests running `Split2QUnitaries` when basis gate set is (non-) discrete""" - qc = QuantumCircuit(3) - qc.sx(0) - qc.t(0) - qc.cx(0, 1) - qc.cx(1, 2) - - orig_pass = Split2QUnitaries() - with patch.object(Split2QUnitaries, "run", wraps=orig_pass.run) as mock_pass: - basis = ["t", "sx", "cx"] - backend = GenericBackendV2(3, basis_gates=basis) - transpile(qc, backend=backend) - transpile(qc, basis_gates=basis) - transpile(qc, target=backend.target) - self.assertFalse(mock_pass.called) - - orig_pass = Split2QUnitaries() - with patch.object(Split2QUnitaries, "run", wraps=orig_pass.run) as mock_pass: - basis = ["rz", "sx", "cx"] - backend = GenericBackendV2(3, basis_gates=basis) - transpile(qc, backend=backend, optimization_level=2) - self.assertTrue(mock_pass.called) - mock_pass.called = False - transpile(qc, basis_gates=basis, optimization_level=2) - self.assertTrue(mock_pass.called) - mock_pass.called = False - transpile(qc, target=backend.target, optimization_level=2) - self.assertTrue(mock_pass.called) - mock_pass.called = False - transpile(qc, backend=backend, optimization_level=3) - self.assertTrue(mock_pass.called) - mock_pass.called = False - transpile(qc, basis_gates=basis, optimization_level=3) - self.assertTrue(mock_pass.called) - def test_optimize_to_nothing(self): """Optimize gates up to fixed point in the default pipeline See https://github.com/Qiskit/qiskit-terra/issues/2035 diff --git a/test/python/transpiler/test_consolidate_blocks.py b/test/python/transpiler/test_consolidate_blocks.py index 8a11af2bd688..1984ad1a3dc4 100644 --- a/test/python/transpiler/test_consolidate_blocks.py +++ b/test/python/transpiler/test_consolidate_blocks.py @@ -17,7 +17,7 @@ import unittest import numpy as np -from qiskit.circuit import QuantumCircuit, QuantumRegister, IfElseOp +from qiskit.circuit import QuantumCircuit, QuantumRegister, IfElseOp, Gate from qiskit.circuit.library import U2Gate, SwapGate, CXGate, CZGate, UnitaryGate from qiskit.converters import circuit_to_dag from qiskit.transpiler.passes import ConsolidateBlocks @@ -553,6 +553,23 @@ def test_inverted_order(self): ) self.assertEqual(expected, actual) + def test_custom_no_target(self): + """Test pass doesn't fail with custom gate.""" + + class MyCustomGate(Gate): + """Custom gate.""" + + def __init__(self): + super().__init__(name="my_custom", num_qubits=2, params=[]) + + qc = QuantumCircuit(2) + qc.append(MyCustomGate(), [0, 1]) + + pm = PassManager([Collect2qBlocks(), ConsolidateBlocks()]) + res = pm.run(qc) + + self.assertEqual(res, qc) + if __name__ == "__main__": unittest.main()