Detect duplicates in QuantumCircuit.compose (#11451)

* Detect duplicates in `QuantumCircuit.compose`

Previously, `QuantumCircuit.compose` would silently attempt to compose
multiple qubits onto the same state, which would fail in weird ways if
any multi-qubit instructions were acting on these collapsed qubits.

* Ensure error-checking happens before any modification

After the move of `QuantumCircuit.data` to Rust-space, some of the state
modification had started happening before the error checking, which
could potentially leave an in-place modification in a partially applied
state if an exception triggered during processing.

This also ensures that the qubits and clbits arguments are checked, even
if the actual data being composed is empty.

* Delay mutation until the last possible time

qiskit/circuit/quantumcircuit.py

@@ -953,9 +953,10 @@ def compose(
                 "Cannot emit a new composed circuit while a control-flow context is active."
             )
 
+        # Avoid mutating `dest` until as much of the error checking as possible is complete, to
+        # avoid an in-place composition getting `self` in a partially mutated state for a simple
+        # error that the user might want to correct in an interactive session.
         dest = self if inplace else self.copy()
-        dest.duration = None
-        dest.unit = "dt"
 
         # 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.
@@ -997,23 +998,9 @@ def compose(
                 "Trying to compose with another QuantumCircuit which has more 'in' edges."
             )
 
-        for gate, cals in other.calibrations.items():
-            dest._calibrations[gate].update(cals)
-
-        dest.global_phase += other.global_phase
-
-        if not other.data:
-            # Nothing left to do. Plus, accessing 'data' here is necessary
-            # to trigger any lazy building since we now access '_data'
-            # directly.
-            return None if inplace else dest
-
-        # The 'qubits' and 'clbits' used for 'dest'.
+        # Maps bits in 'other' to bits in 'dest'.
         mapped_qubits: list[Qubit]
         mapped_clbits: list[Clbit]
-
-        # Maps bits in 'other' to bits in 'dest'. Used only for
-        # adjusting bits in variables (e.g. condition and target).
         edge_map: dict[Qubit | Clbit, Qubit | Clbit] = {}
         if qubits is None:
             mapped_qubits = dest.qubits
@@ -1025,6 +1012,10 @@ def compose(
                     f"Number of items in qubits parameter ({len(mapped_qubits)}) does not"
                     f" match number of qubits in the circuit ({len(other.qubits)})."
                 )
+            if len(set(mapped_qubits)) != len(mapped_qubits):
+                raise CircuitError(
+                    f"Duplicate qubits referenced in 'qubits' parameter: '{mapped_qubits}'"
+                )
             edge_map.update(zip(other.qubits, mapped_qubits))
 
         if clbits is None:
@@ -1037,8 +1028,25 @@ def compose(
                     f"Number of items in clbits parameter ({len(mapped_clbits)}) does not"
                     f" match number of clbits in the circuit ({len(other.clbits)})."
                 )
+            if len(set(mapped_clbits)) != len(mapped_clbits):
+                raise CircuitError(
+                    f"Duplicate clbits referenced in 'clbits' parameter: '{mapped_clbits}'"
+                )
             edge_map.update(zip(other.clbits, dest.cbit_argument_conversion(clbits)))
 
+        for gate, cals in other.calibrations.items():
+            dest._calibrations[gate].update(cals)
+
+        dest.duration = None
+        dest.unit = "dt"
+        dest.global_phase += other.global_phase
+
+        if not other.data:
+            # Nothing left to do. Plus, accessing 'data' here is necessary
+            # to trigger any lazy building since we now access '_data'
+            # directly.
+            return None if inplace else dest
+
         variable_mapper = _classical_resource_map.VariableMapper(
             dest.cregs, edge_map, dest.add_register
         )

releasenotes/notes/fix-circuit-compose-duplicate-59de6c1c51f58e54.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    :meth:`.QuantumCircuit.compose` will now correctly raise a :exc:`.CircuitError` when there are
+    duplicates in the ``qubits`` or ``clbits`` arguments.

test/python/circuit/test_compose.py

@@ -31,6 +31,7 @@
     Instruction,
     CASE_DEFAULT,
     SwitchCaseOp,
+    CircuitError,
 )
 from qiskit.circuit.library import HGate, RZGate, CXGate, CCXGate, TwoLocal
 from qiskit.circuit.classical import expr
@@ -880,6 +881,16 @@ def test_expr_target_is_mapped(self):
 
         self.assertEqual(dest, expected)
 
+    def test_rejects_duplicate_bits(self):
+        """Test that compose rejects duplicates in either qubits or clbits."""
+        base = QuantumCircuit(5, 5)
+
+        attempt = QuantumCircuit(2, 2)
+        with self.assertRaisesRegex(CircuitError, "Duplicate qubits"):
+            base.compose(attempt, [1, 1], [0, 1])
+        with self.assertRaisesRegex(CircuitError, "Duplicate clbits"):
+            base.compose(attempt, [0, 1], [1, 1])
+
 
 if __name__ == "__main__":
     unittest.main()