Fix ConsolidateBlocks pass for collecting non-CX KAK gate (#14417) (#14424)

A bug was introduced into the ConsolidateBlocks pass in #13884 around
how the KAK gate name was passed to rust from the Python pass. The
consolidate blocks internal logic around whether to consolidate a pass
or not is based on the estimate from the selected decomposer of whether
the estimated number of gates used to synthesis the unitary would exceed
the number of basis gates in the block. To do this the pass counts the
gate with the synthesis target name in the block. However, in the case
of the decomposer being the TwoQubitBasisDecomposer the name being
used for this look wasn't the name of the gate, but instead was an
internal sentinel value string "USER_GATE" which is used to handle
arbitrary user gate definitions in the decomposer. This led to the
consolidate blocks pass missing opportunities for consolidation. This
commit fixes the oversight so that the name we use for making this
determination is the actual basis gate name and the pass functions
correctly.

Fixes #14413

(cherry picked from commit 4e9231d)

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

Author: mergify[bot]

qiskit/transpiler/passes/optimization/consolidate_blocks.py

@@ -91,6 +91,7 @@ def __init__(
         """
         super().__init__()
         self.basis_gates = None
+        self.basis_gate_name = None
         # Bypass target if it doesn't contain any basis gates (i.e. it's a _FakeTarget), as this
         # not part of the official target model.
         self.target = target if target is not None and len(target.operation_names) > 0 else None
@@ -99,21 +100,25 @@ def __init__(
         self.force_consolidate = force_consolidate
         if kak_basis_gate is not None:
             self.decomposer = TwoQubitBasisDecomposer(kak_basis_gate)
+            self.basis_gate_name = kak_basis_gate.name
         elif basis_gates is not None:
             kak_gates = KAK_GATE_NAMES.keys() & (basis_gates or [])
             kak_param_gates = KAK_GATE_PARAM_NAMES.keys() & (basis_gates or [])
             if kak_param_gates:
                 self.decomposer = TwoQubitControlledUDecomposer(
                     KAK_GATE_PARAM_NAMES[list(kak_param_gates)[0]]
                 )
+                self.basis_gate_name = list(kak_param_gates)[0]
             elif kak_gates:
                 self.decomposer = TwoQubitBasisDecomposer(
                     KAK_GATE_NAMES[list(kak_gates)[0]], basis_fidelity=approximation_degree or 1.0
                 )
+                self.basis_gate_name = list(kak_gates)[0]
             else:
                 self.decomposer = None
         else:
             self.decomposer = TwoQubitBasisDecomposer(CXGate())
+            self.basis_gate_name = "cx"
 
     def run(self, dag):
         """Run the ConsolidateBlocks pass on `dag`.
@@ -134,7 +139,7 @@ def run(self, dag):
         consolidate_blocks(
             dag,
             self.decomposer._inner_decomposer,
-            self.decomposer.gate_name,
+            self.basis_gate_name,
             self.force_consolidate,
             target=self.target,
             basis_gates=self.basis_gates,

releasenotes/notes/fix-consolidate-blocks-e2421dd2408403eb.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixed an issue in the :class:`.ConsolidateBlocks` transpiler pass where it
+    would fail to consolidate some blocks if the KAK gate selected (either
+    directly or via the target) is supercontrolled and not :class:`.CXGate`.
+    Fixed `#14413 <https://github.com/Qiskit/qiskit/issues/14413>`__

test/python/transpiler/test_consolidate_blocks.py

@@ -14,7 +14,6 @@
 Tests for the ConsolidateBlocks transpiler pass.
 """
 
-import unittest
 import numpy as np
 from ddt import ddt, data
 
@@ -24,10 +23,12 @@
     SwapGate,
     CXGate,
     CZGate,
+    ECRGate,
     UnitaryGate,
     SXGate,
     XGate,
     RZGate,
+    RZZGate,
 )
 from qiskit.converters import circuit_to_dag
 from qiskit.quantum_info.operators import Operator
@@ -352,12 +353,13 @@ def test_single_gate_block_outside_basis(self):
         expected.unitary(np.array([[1, 0, 0, 0], [0, 0, 1, 0], [0, 1, 0, 0], [0, 0, 0, 1]]), [0, 1])
         self.assertEqual(expected, pass_manager.run(qc))
 
-    def test_single_gate_block_outside_basis_with_target(self):
+    @data(CXGate, CZGate, ECRGate)
+    def test_single_gate_block_outside_basis_with_target(self, basis_gate):
         """Test a gate outside basis defined in target gets converted."""
         qc = QuantumCircuit(2)
         target = Target(num_qubits=2)
         # Add ideal basis gates to all qubits
-        target.add_instruction(CXGate())
+        target.add_instruction(basis_gate())
         qc.swap(0, 1)
         consolidate_block_pass = ConsolidateBlocks(target=target)
         pass_manager = PassManager()
@@ -367,12 +369,13 @@ def test_single_gate_block_outside_basis_with_target(self):
         expected.unitary(np.array([[1, 0, 0, 0], [0, 0, 1, 0], [0, 1, 0, 0], [0, 0, 0, 1]]), [0, 1])
         self.assertEqual(expected, pass_manager.run(qc))
 
-    def test_single_gate_block_outside_local_basis_with_target(self):
+    @data(CXGate, CZGate, ECRGate)
+    def test_single_gate_block_outside_local_basis_with_target(self, basis_gate):
         """Test that a gate in basis but outside valid qubits is treated as outside basis with target."""
         qc = QuantumCircuit(2)
         target = Target(num_qubits=2)
-        # Add ideal cx to (1, 0) only
-        target.add_instruction(CXGate(), {(1, 0): None})
+        # Add ideal basis to (1, 0) only
+        target.add_instruction(basis_gate(), {(1, 0): None})
         qc.cx(0, 1)
         consolidate_block_pass = ConsolidateBlocks(target=target)
         pass_manager = PassManager()
@@ -382,15 +385,16 @@ def test_single_gate_block_outside_local_basis_with_target(self):
         expected.unitary(np.array([[1, 0, 0, 0], [0, 0, 0, 1], [0, 0, 1, 0], [0, 1, 0, 0]]), [0, 1])
         self.assertEqual(expected, pass_manager.run(qc))
 
-    def test_single_gate_block_outside_target_with_matching_basis_gates(self):
+    @data("cx", "ecr", "cz")
+    def test_single_gate_block_outside_target_with_matching_basis_gates(self, basis_gate):
         """Ensure the target is the source of truth with basis_gates also set."""
         qc = QuantumCircuit(2)
         target = Target(num_qubits=2)
         # Add ideal cx to (1, 0) only
         target.add_instruction(SwapGate())
         qc.swap(0, 1)
         consolidate_block_pass = ConsolidateBlocks(
-            basis_gates=["id", "cx", "rz", "sx", "x"], target=target
+            basis_gates=["id", basis_gate, "rz", "sx", "x"], target=target
         )
         pass_manager = PassManager()
         pass_manager.append(Collect2qBlocks())
@@ -669,6 +673,21 @@ def test_collect_and_synthesize_rzz(self, basis_gates):
         tqc = pm.run(qc)
         self.assertEqual(tqc.count_ops()["rzz"], 1)
 
+    @data(CXGate, CZGate, ECRGate)
+    def test_rzz_collection(self, basis_gate):
+        """Test that a parameterized gate outside the target is consolidated."""
+        phi = Parameter("phi")
+        target = Target(num_qubits=2)
+        target.add_instruction(SXGate(), {(0,): None, (1,): None})
+        target.add_instruction(XGate(), {(0,): None, (1,): None})
+        target.add_instruction(RZGate(phi), {(0,): None, (1,): None})
+        target.add_instruction(basis_gate(), {(0, 1): None, (1, 0): None})
+        consolidate_pass = ConsolidateBlocks(target=target)
 
-if __name__ == "__main__":
-    unittest.main()
+        for angle in [np.pi / 2, np.pi]:
+            qc = QuantumCircuit(2)
+            qc.rzz(angle, 0, 1)
+            res = consolidate_pass(qc)
+            expected = QuantumCircuit(2)
+            expected.unitary(np.asarray(RZZGate(angle)), [0, 1])
+            self.assertEqual(res, expected)