Fix QuantumCircuit.compose on unusual types and registers

qiskit/circuit/quantumcircuit.py

@@ -769,8 +769,8 @@ def control(
     def compose(
         self,
         other: Union["QuantumCircuit", Instruction],
-        qubits: Optional[Sequence[Union[Qubit, int]]] = None,
-        clbits: Optional[Sequence[Union[Clbit, int]]] = None,
+        qubits: Optional[Union[QubitSpecifier, Sequence[QubitSpecifier]]] = None,
+        clbits: Optional[Union[ClbitSpecifier, Sequence[ClbitSpecifier]]] = None,
         front: bool = False,
         inplace: bool = False,
         wrap: bool = False,
@@ -840,98 +840,100 @@ def compose(
                 "Cannot emit a new composed circuit while a control-flow context is active."
             )
 
-        if inplace:
-            dest = self
-        else:
-            dest = self.copy()
+        dest = self if inplace else self.copy()
 
-        # If self does not have any cregs but other does then we allow
-        # that and add the registers to the output dest
+        # As a special case, allow composing some clbits onto no clbits - normally the destination
+        # has to be strictly larger. This allows composing final measurements onto unitary circuits.
         if isinstance(other, QuantumCircuit):
             if not self.clbits and other.clbits:
                 dest.add_bits(other.clbits)
                 for reg in other.cregs:
                     dest.add_register(reg)
 
-        if wrap:
-            try:
-                other = other.to_gate()
-            except QiskitError:
-                other = other.to_instruction()
+        if wrap and isinstance(other, QuantumCircuit):
+            other = (
+                other.to_gate()
+                if all(isinstance(ins.operation, Gate) for ins in other.data)
+                else other.to_instruction()
+            )
 
         if not isinstance(other, QuantumCircuit):
             if qubits is None:
-                qubits = list(range(other.num_qubits))
-
+                qubits = self.qubits[: other.num_qubits]
             if clbits is None:
-                clbits = list(range(other.num_clbits))
-
+                clbits = self.clbits[: other.num_clbits]
             if front:
-                dest.data.insert(0, CircuitInstruction(other, qubits, clbits))
+                old_data = list(dest.data)
+                dest.clear()
+                dest.append(other, qubits, clbits)
+                for instruction in old_data:
+                    dest._append(instruction)
             else:
                 dest.append(other, qargs=qubits, cargs=clbits)
-
             if inplace:
                 return None
             return dest
 
-        instrs = other.data
-
         if other.num_qubits > dest.num_qubits or other.num_clbits > dest.num_clbits:
             raise CircuitError(
                 "Trying to compose with another QuantumCircuit which has more 'in' edges."
             )
 
         # number of qubits and clbits must match number in circuit or None
-        identity_qubit_map = dict(zip(other.qubits, dest.qubits))
-        identity_clbit_map = dict(zip(other.clbits, dest.clbits))
+        edge_map = {}
         if qubits is None:
-            qubit_map = identity_qubit_map
-        elif len(qubits) != len(other.qubits):
-            raise CircuitError(
-                f"Number of items in qubits parameter ({len(qubits)}) does not"
-                f" match number of qubits in the circuit ({len(other.qubits)})."
-            )
+            edge_map.update(zip(other.qubits, dest.qubits))
         else:
-            qubit_map = {
-                other.qubits[i]: (self.qubits[q] if isinstance(q, int) else q)
-                for i, q in enumerate(qubits)
-            }
+            mapped_qubits = dest.qbit_argument_conversion(qubits)
+            if len(mapped_qubits) != len(other.qubits):
+                raise CircuitError(
+                    f"Number of items in qubits parameter ({len(mapped_qubits)}) does not"
+                    f" match number of qubits in the circuit ({len(other.qubits)})."
+                )
+            edge_map.update(zip(other.qubits, mapped_qubits))
+
         if clbits is None:
-            clbit_map = identity_clbit_map
-        elif len(clbits) != len(other.clbits):
-            raise CircuitError(
-                f"Number of items in clbits parameter ({len(clbits)}) does not"
-                f" match number of clbits in the circuit ({len(other.clbits)})."
-            )
+            edge_map.update(zip(other.clbits, dest.clbits))
         else:
-            clbit_map = {
-                other.clbits[i]: (self.clbits[c] if isinstance(c, int) else c)
-                for i, c in enumerate(clbits)
-            }
-
-        edge_map = {**qubit_map, **clbit_map} or {**identity_qubit_map, **identity_clbit_map}
+            mapped_clbits = dest.cbit_argument_conversion(clbits)
+            if len(mapped_clbits) != len(other.clbits):
+                raise CircuitError(
+                    f"Number of items in clbits parameter ({len(mapped_clbits)}) does not"
+                    f" match number of clbits in the circuit ({len(other.clbits)})."
+                )
+            edge_map.update(zip(other.clbits, dest.cbit_argument_conversion(clbits)))
 
         mapped_instrs = []
-        for instr in instrs:
+        condition_register_map = {}
+        for instr in other.data:
             n_qargs = [edge_map[qarg] for qarg in instr.qubits]
             n_cargs = [edge_map[carg] for carg in instr.clbits]
-            n_instr = instr.operation.copy()
-
-            if getattr(instr.operation, "condition", None) is not None:
-                from qiskit.dagcircuit import DAGCircuit  # pylint: disable=cyclic-import
+            n_op = instr.operation.copy()
 
-                n_instr.condition = DAGCircuit._map_condition(
-                    edge_map, instr.operation.condition, self.cregs
-                )
+            # Map their registers over to ours, adding an extra one if there's no exact match.
+            if getattr(n_op, "condition", None) is not None:
+                target, value = n_op.condition
+                if isinstance(target, Clbit):
+                    n_op.condition = (edge_map[target], value)
+                else:
+                    if target.name not in condition_register_map:
+                        mapped_bits = [edge_map[bit] for bit in target]
+                        for our_creg in dest.cregs:
+                            if mapped_bits == list(our_creg):
+                                new_target = our_creg
+                                break
+                        else:
+                            new_target = ClassicalRegister(bits=[edge_map[bit] for bit in target])
+                            dest.add_register(new_target)
+                        condition_register_map[target.name] = new_target
+                    n_op.condition = (condition_register_map[target.name], value)
 
-            mapped_instrs.append(CircuitInstruction(n_instr, n_qargs, n_cargs))
+            mapped_instrs.append(CircuitInstruction(n_op, n_qargs, n_cargs))
 
         if front:
             # adjust new instrs before original ones and update all parameters
             mapped_instrs += dest.data
-            dest.data.clear()
-            dest._parameter_table.clear()
+            dest.clear()
         append = dest._control_flow_scopes[-1].append if dest._control_flow_scopes else dest._append
         for instr in mapped_instrs:
             append(instr)

releasenotes/notes/fix-compose-35d2fdbe5b052bca.yaml

@@ -0,0 +1,16 @@
+---
+fixes:
+  - |
+    The method :meth:`.QuantumCircuit.compose` now accepts the same set of qubit and clbit
+    specifiers as other :class:`.QuantumCircuit` methods, such as :meth:`~.QuantumCircuit.append`.
+    This means, for example, that Numpy integers will work.  Fixed `#8691
+    <https://github.com/Qiskit/qiskit-terra/issues/8691>`__.
+  - |
+    Fixed :meth:`.QuantumCircuit.compose` incorrectly mapping registers in conditions on the given
+    circuit to complete registers on the base.  Previously, the mapping was very imprecise; the bits
+    used within each condition were not subject to the mapping, and instead an inaccurate attempt was
+    made to find a corresponding register.  This could also result in a condition on a smaller register
+    being expanded to be on a larger register, which is not a valid transformation.  Now, a
+    condition on a single bit or a register will be composed to be on precisely the bits as defined
+    by the ``clbits`` argument.  A new aliasing register will be added to the base circuit to
+    facilitate this, if necessary.  Fixed `#6583 <https://github.com/Qiskit/qiskit-terra/issues/8691>__`.

test/python/circuit/test_compose.py

@@ -16,6 +16,8 @@
 
 import unittest
 
+import numpy as np
+
 from qiskit import transpile
 from qiskit.pulse import Schedule
 from qiskit.circuit import (
@@ -107,6 +109,30 @@ def test_compose_inorder(self):
         circuit_composed = self.circuit_left.compose(circuit_right, inplace=False)
         self.assertEqual(circuit_composed, circuit_expected)
 
+    def test_compose_inorder_unusual_types(self):
+        """Test that composition works in order, using Numpy integer types as well as regular
+        integer types.  In general, it should be permissible to use any of the same `QubitSpecifier`
+        types (or similar for `Clbit`) that `QuantumCircuit.append` uses."""
+        qreg = QuantumRegister(5, "rqr")
+        creg = ClassicalRegister(2, "rcr")
+        circuit_right = QuantumCircuit(qreg, creg)
+        circuit_right.cx(qreg[0], qreg[3])
+        circuit_right.x(qreg[1])
+        circuit_right.y(qreg[2])
+        circuit_right.z(qreg[4])
+        circuit_right.measure([0, 1], [0, 1])
+
+        circuit_expected = self.circuit_left.copy()
+        circuit_expected.cx(self.left_qubit0, self.left_qubit3)
+        circuit_expected.x(self.left_qubit1)
+        circuit_expected.y(self.left_qubit2)
+        circuit_expected.z(self.left_qubit4)
+        circuit_expected.measure(self.left_qubit0, self.left_clbit0)
+        circuit_expected.measure(self.left_qubit1, self.left_clbit1)
+
+        circuit_composed = self.circuit_left.compose(circuit_right, np.arange(5), slice(0, 2))
+        self.assertEqual(circuit_composed, circuit_expected)
+
     def test_compose_inorder_inplace(self):
         """Composing two circuits of the same width, default order, inplace.
 
@@ -397,9 +423,9 @@ def test_compose_conditional(self):
         lqr_2_1: ──┤ X ├────┤ X ├────┼───┤M├─╫─
                    └───┘    └─┬─┘    │   └╥┘ ║
                            ┌──┴──┐┌──┴──┐ ║  ║
-        lcr_0: ════════════╡     ╞╡     ╞═╩══╬═
-                           │ = 3 ││ = 3 │    ║
-        lcr_1: ════════════╡     ╞╡     ╞════╩═
+        lcr_0: ════════════╡     ╞╡     ╞═╬══╩═
+                           │ = 3 ││ = 3 │ ║
+        lcr_1: ════════════╡     ╞╡     ╞═╩════
                            └─────┘└─────┘
         """
         qreg = QuantumRegister(2, "rqr")
@@ -411,16 +437,47 @@ def test_compose_conditional(self):
         circuit_right.measure(qreg, creg)
 
         # permuted subset of qubits and clbits
-        circuit_composed = self.circuit_left.compose(circuit_right, qubits=[1, 4], clbits=[1, 0])
+        circuit_composed = self.circuit_left.compose(circuit_right, qubits=[1, 4], clbits=[0, 1])
 
         circuit_expected = self.circuit_left.copy()
         circuit_expected.x(self.left_qubit4).c_if(*self.condition)
         circuit_expected.h(self.left_qubit1).c_if(*self.condition)
-        circuit_expected.measure(self.left_qubit4, self.left_clbit0)
-        circuit_expected.measure(self.left_qubit1, self.left_clbit1)
+        circuit_expected.measure(self.left_qubit1, self.left_clbit0)
+        circuit_expected.measure(self.left_qubit4, self.left_clbit1)
 
         self.assertEqual(circuit_composed, circuit_expected)
 
+    def test_compose_conditional_no_match(self):
+        """Test that compose correctly maps registers in conditions to the new circuit, even when
+        there are no matching registers in the destination circuit.
+
+        Regression test of gh-6583 and gh-6584."""
+        right = QuantumCircuit(QuantumRegister(3), ClassicalRegister(1), ClassicalRegister(1))
+        right.h(1)
+        right.cx(1, 2)
+        right.cx(0, 1)
+        right.h(0)
+        right.measure([0, 1], [0, 1])
+        right.z(2).c_if(right.cregs[0], 1)
+        right.x(2).c_if(right.cregs[1], 1)
+        test = QuantumCircuit(3, 3).compose(right, range(3), range(2))
+        z = next(ins.operation for ins in test.data[::-1] if ins.operation.name == "z")
+        x = next(ins.operation for ins in test.data[::-1] if ins.operation.name == "x")
+        # The registers should have been mapped, including the bits inside them.  Unlike the
+        # previous test, there are no matching registers in the destination circuit, so the
+        # composition needs to add new registers (bit groupings) over the existing mapped bits.
+        self.assertIsNot(z.condition, None)
+        self.assertIsInstance(z.condition[0], ClassicalRegister)
+        self.assertEqual(len(z.condition[0]), len(right.cregs[0]))
+        self.assertIs(z.condition[0][0], test.clbits[0])
+        self.assertEqual(z.condition[1], 1)
+        #
+        self.assertIsNot(x.condition, None)
+        self.assertIsInstance(x.condition[0], ClassicalRegister)
+        self.assertEqual(len(x.condition[0]), len(right.cregs[1]))
+        self.assertEqual(z.condition[1], 1)
+        self.assertIs(x.condition[0][0], test.clbits[1])
+
     def test_compose_gate(self):
         """Composing with a gate.
 

test/python/dagcircuit/test_compose.py

@@ -16,7 +16,6 @@
 
 from qiskit.circuit import QuantumRegister, ClassicalRegister, QuantumCircuit
 from qiskit.converters import circuit_to_dag, dag_to_circuit
-from qiskit.dagcircuit.exceptions import DAGCircuitError
 from qiskit.test import QiskitTestCase
 from qiskit.pulse import Schedule
 from qiskit.circuit.gate import Gate
@@ -424,31 +423,6 @@ def test_compose_condition_multiple_classical(self):
 
         self.assertEqual(dag_composed, dag_expected)
 
-    def test_compose_raises_if_splitting_condition_creg(self):
-        """Verify compose raises if a condition is mapped to more than one creg.
-
-                             ┌───┐
-        q_0:           q_0: ─┤ H ├─
-                             └─┬─┘
-        c0: 1/  +           ┌──┴──┐   = DAGCircuitError
-                       c: 2/╡ = 2 ╞
-        c1: 1/              └─────┘
-        """
-
-        qreg = QuantumRegister(1)
-        creg1 = ClassicalRegister(1)
-        creg2 = ClassicalRegister(1)
-
-        circuit_left = QuantumCircuit(qreg, creg1, creg2)
-
-        wide_creg = ClassicalRegister(2)
-
-        circuit_right = QuantumCircuit(qreg, wide_creg)
-        circuit_right.h(0).c_if(wide_creg, 2)
-
-        with self.assertRaisesRegex(DAGCircuitError, "more than one creg"):
-            circuit_left.compose(circuit_right)
-
     def test_compose_calibrations(self):
         """Test that compose carries over the calibrations."""
         dag_cal = QuantumCircuit(1)