Fix QuantumCircuit.decompose for high-level objects (#13311)

* Fix decompose for HLS objects

* add reno

* rm old comments

qiskit/circuit/quantumcircuit.py

@@ -3229,40 +3229,33 @@ def to_gate(
 
     def decompose(
         self,
-        gates_to_decompose: Type[Gate] | Sequence[Type[Gate]] | Sequence[str] | str | None = None,
+        gates_to_decompose: (
+            str | Type[Instruction] | Sequence[str | Type[Instruction]] | None
+        ) = None,
         reps: int = 1,
-    ) -> "QuantumCircuit":
-        """Call a decomposition pass on this circuit,
-        to decompose one level (shallow decompose).
+    ) -> typing.Self:
+        """Call a decomposition pass on this circuit, to decompose one level (shallow decompose).
 
         Args:
-            gates_to_decompose (type or str or list(type, str)): Optional subset of gates
-                to decompose. Can be a gate type, such as ``HGate``, or a gate name, such
-                as 'h', or a gate label, such as 'My H Gate', or a list of any combination
-                of these. If a gate name is entered, it will decompose all gates with that
-                name, whether the gates have labels or not. Defaults to all gates in circuit.
-            reps (int): Optional number of times the circuit should be decomposed.
+            gates_to_decompose: Optional subset of gates to decompose. Can be a gate type, such as
+                ``HGate``, or a gate name, such as "h", or a gate label, such as "My H Gate", or a
+                list of any combination of these. If a gate name is entered, it will decompose all
+                gates with that name, whether the gates have labels or not. Defaults to all gates in
+                the circuit.
+            reps: Optional number of times the circuit should be decomposed.
                 For instance, ``reps=2`` equals calling ``circuit.decompose().decompose()``.
-                can decompose specific gates specific time
 
         Returns:
             QuantumCircuit: a circuit one level decomposed
         """
         # pylint: disable=cyclic-import
         from qiskit.transpiler.passes.basis.decompose import Decompose
-        from qiskit.transpiler.passes.synthesis import HighLevelSynthesis
         from qiskit.converters.circuit_to_dag import circuit_to_dag
         from qiskit.converters.dag_to_circuit import dag_to_circuit
 
         dag = circuit_to_dag(self, copy_operations=True)
 
-        if gates_to_decompose is None:
-            # We should not rewrite the circuit using HLS when we have gates_to_decompose,
-            # or else HLS will rewrite all objects with available plugins (e.g., Cliffords,
-            # PermutationGates, and now also MCXGates)
-            dag = HighLevelSynthesis().run(dag)
-
-        pass_ = Decompose(gates_to_decompose)
+        pass_ = Decompose(gates_to_decompose, apply_synthesis=True)
         for _ in range(reps):
             dag = pass_.run(dag)
 

qiskit/transpiler/passes/basis/decompose.py

@@ -11,30 +11,42 @@
 # that they have been altered from the originals.
 
 """Expand a gate in a circuit using its decomposition rules."""
-from typing import Type, Union, List, Optional
+
+from __future__ import annotations
+
+from collections.abc import Sequence
+from typing import Type
 from fnmatch import fnmatch
 
 from qiskit.transpiler.basepasses import TransformationPass
+from qiskit.dagcircuit.dagnode import DAGOpNode
 from qiskit.dagcircuit.dagcircuit import DAGCircuit
 from qiskit.converters.circuit_to_dag import circuit_to_dag
-from qiskit.circuit.gate import Gate
+from qiskit.circuit.instruction import Instruction
+
+from ..synthesis import HighLevelSynthesis
 
 
 class Decompose(TransformationPass):
     """Expand a gate in a circuit using its decomposition rules."""
 
     def __init__(
         self,
-        gates_to_decompose: Optional[Union[Type[Gate], List[Type[Gate]], List[str], str]] = None,
+        gates_to_decompose: (
+            str | Type[Instruction] | Sequence[str | Type[Instruction]] | None
+        ) = None,
+        apply_synthesis: bool = False,
     ) -> None:
-        """Decompose initializer.
-
+        """
         Args:
             gates_to_decompose: optional subset of gates to be decomposed,
                 identified by gate label, name or type. Defaults to all gates.
+            apply_synthesis: If ``True``, run :class:`.HighLevelSynthesis` to synthesize operations
+                that do not have a definition attached.
         """
         super().__init__()
         self.gates_to_decompose = gates_to_decompose
+        self.apply_synthesis = apply_synthesis
 
     def run(self, dag: DAGCircuit) -> DAGCircuit:
         """Run the Decompose pass on `dag`.
@@ -45,13 +57,26 @@ def run(self, dag: DAGCircuit) -> DAGCircuit:
         Returns:
             output dag where ``gate`` was expanded.
         """
+        # We might use HLS to synthesize objects that do not have a definition
+        hls = HighLevelSynthesis() if self.apply_synthesis else None
+
         # Walk through the DAG and expand each non-basis node
         for node in dag.op_nodes():
-            if self._should_decompose(node):
-                if getattr(node.op, "definition", None) is None:
-                    continue
-                # TODO: allow choosing among multiple decomposition rules
+            # Check in self.gates_to_decompose if the operation should be decomposed
+            if not self._should_decompose(node):
+                continue
+
+            if getattr(node.op, "definition", None) is None:
+                # if we try to synthesize, turn the node into a DAGCircuit and run HLS
+                if self.apply_synthesis:
+                    node_as_dag = _node_to_dag(node)
+                    synthesized = hls.run(node_as_dag)
+                    dag.substitute_node_with_dag(node, synthesized)
+
+                # else: no definition and synthesis not enabled, so we do nothing
+            else:
                 rule = node.op.definition.data
+
                 if (
                     len(rule) == 1
                     and len(node.qargs) == len(rule[0].qubits) == 1  # to preserve gate order
@@ -66,9 +91,8 @@ def run(self, dag: DAGCircuit) -> DAGCircuit:
 
         return dag
 
-    def _should_decompose(self, node) -> bool:
-        """Call a decomposition pass on this circuit,
-        to decompose one level (shallow decompose)."""
+    def _should_decompose(self, node: DAGOpNode) -> bool:
+        """Call a decomposition pass on this circuit to decompose one level (shallow decompose)."""
         if self.gates_to_decompose is None:  # check if no gates given
             return True
 
@@ -96,3 +120,12 @@ def _should_decompose(self, node) -> bool:
             return True
         else:
             return False
+
+
+def _node_to_dag(node: DAGOpNode) -> DAGCircuit:
+    dag = DAGCircuit()
+    dag.add_qubits(node.qargs)
+    dag.add_clbits(node.cargs)
+
+    dag.apply_operation_back(node.op, node.qargs, node.cargs)
+    return dag

releasenotes/notes/fix-decompose-hls-5019793177136024.yaml

@@ -0,0 +1,42 @@
+---
+features_circuits:
+  - |
+    Added a new argument ``"apply_synthesis"`` to :class:`.Decompose`, which allows
+    the transpiler pass to apply high-level synthesis to decompose objects that are only
+    defined by a synthesis routine. For example::
+
+      from qiskit import QuantumCircuit
+      from qiskit.quantum_info import Clifford
+      from qiskit.transpiler.passes import Decompose
+
+      cliff = Clifford(HGate())
+      circuit = QuantumCircuit(1)
+      circuit.append(cliff, [0])
+
+      # Clifford has no .definition, it is only defined by synthesis
+      nothing_happened = Decompose()(circuit)
+
+      # this internally runs the HighLevelSynthesis pass to decompose the Clifford
+      decomposed = Decompose(apply_synthesis=True)(circuit)
+
+fixes:
+  - |
+    Fixed a bug in :meth:`.QuantumCircuit.decompose` where objects that could be synthesized
+    with :class:`.HighLevelSynthesis` were first synthesized and then decomposed immediately
+    (i.e., they were decomposed twice instead of once). This affected, e.g., :class:`.MCXGate`
+    or :class:`.Clifford`, among others.
+  - |
+    Fixed a bug in :meth:`.QuantumCircuit.decompose`, where high-level objects without a definition
+    were not decomposed if they were explicitly set via the ``"gates_to_decompose"`` argument.
+    For example, previously the following did not perform a decomposition but now works as
+    expected::
+
+      from qiskit import QuantumCircuit
+      from qiskit.quantum_info import Clifford
+      from qiskit.transpiler.passes import Decompose
+
+      cliff = Clifford(HGate())
+      circuit = QuantumCircuit(1)
+      circuit.append(cliff, [0])
+
+      decomposed = Decompose(gates_to_decompose=["clifford"])(circuit)

test/python/transpiler/test_decompose.py

@@ -18,7 +18,7 @@
 from qiskit.transpiler.passes import Decompose
 from qiskit.converters import circuit_to_dag
 from qiskit.circuit.library import HGate, CCXGate, U2Gate
-from qiskit.quantum_info.operators import Operator
+from qiskit.quantum_info.operators import Operator, Clifford
 from test import QiskitTestCase  # pylint: disable=wrong-import-order
 
 
@@ -317,3 +317,34 @@ def test_decompose_single_qubit_clbit(self):
         decomposed = circuit.decompose()
 
         self.assertEqual(decomposed, block)
+
+    def test_decompose_synthesis(self):
+        """Test a high-level object with only a synthesis and no definition is correctly decomposed."""
+        qc = QuantumCircuit(1)
+        qc.h(0)
+        cliff = Clifford(qc)
+
+        bigger = QuantumCircuit(1)
+        bigger.append(cliff, [0])
+
+        decomposed = bigger.decompose()
+
+        self.assertEqual(qc, decomposed)
+
+    def test_specify_hls_object(self):
+        """Test specifying an HLS object by name works."""
+        qc = QuantumCircuit(1)
+        qc.h(0)
+        cliff = Clifford(qc)
+
+        bigger = QuantumCircuit(1)
+        bigger.append(cliff, [0])
+        bigger.h(0)  # add another gate that should remain unaffected, but has a definition
+
+        decomposed = bigger.decompose(gates_to_decompose=["clifford"])
+
+        expected = QuantumCircuit(1)
+        expected.h(0)
+        expected.h(0)
+
+        self.assertEqual(expected, decomposed)