Remove init peephole optimization discrete basis check (#12898) (#12920)

* 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 70c2f78)

Co-authored-by: Matthew Treinish <[email protected]>

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.

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

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.

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

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()