Change data type of QuantumCircuit.data elements (Qiskit#8093)

* Convert QuantumCircuit.data items to CircuitInstruction

This introduces a new, lightweight class `CircuitInstruction` the
encapsulates the previous 3-tuple that made up the items of
`QuantumCircuit.data`.  It includes a legacy API that makes the object
appear to be the old 3-tuple when used as if it were that tuple, and the
relatively small changeset for this commit is an approximate indication
that these should mostly be suitable.  There will be significant
performance penalties for accessing the legacy interface, however, due
to some internal type conversions, so it is a matter of priority to
convert internal usage to the new dotted-attribute access form.

The only places necessary to change across Terra were those where the
data elements were compared by referential equality to expected values
(such as in the control-flow builders), and places that were invalidly
accessing the private attribute `QuantumCircuit._data`.  All these are
updated.

`ParameterTable` should in the future be changed to operate over this
entire context, but that was beyond the scope of the initial commit.

* Convert all usage of data tuple to CircuitInstruction

This previous commit is the minimal set of changes needed for the entire
test suite to pass, but large parts of Terra and the test suite were
still using the backwards-compatibility shims when dealing with
`QuantumCircuit.data`.  This changes over every single usage within
Terra to use the new dotted-access form, rather than the tuple indexing
or iteration that previously existed.

The internal types of the `qargs` and `cargs` in `DAGDepNode` and
`DAGOpNode` are also changed to be `tuple` now; previously they were
semi-undefined, and in some cases were not consistent, but not caught in
the test suite.  Since changing the type of the `qubits` parameter in
`CircuitInstruction` to be tuple (unless using the
backwards-compatibility shims), this caused a lot of failures where the
value was being assigned directly.  Normalising to `tuple` has the same
benefit as it does in the circuit instruction - less copying overhead,
and no worrying about multiple writable references to the same object.

By far the largest set of changes are in the test suite; we do a lot of
comparing things for exact equality, and since the type is now changed
from `list` to `tuple`, all the tests had to be updated.  This isn't
expected to have much impact on user code, but changing the type to an
immutable one has large benefits for us when copying these data
structures.

* Correct bad version in comment

* Simplify unnecessary if/else

* Update remnant legacy interface uses

Some uses of the legacy interface related to directly setting
`QuantumCircuit.data` had slipped through the cracks.  A bug in
`astroid` caused its inference to fail, and think that these settings
made the scalar type of `QuantumCircuit.data` the old 3-tuple after it
saw these settings.  Since there's a property setter for `data`, that's
a bug in `astroid`, but it still turned up some useful additional cases
that were useful to correct.

* Remove accidental blank line

* Use QuantumCircuit.copy_empty_like in Instruction constructors

* Correct global phase in Instruction.inverse

* Shut pylint up about consider-using-generator

pylint isn't actually correct in this case.  `tuple(<list comp>)` is
more efficient than `tuple(<generator>)` (certainly as of Python 3.10),
because Python still needs to use a temporary vector-like expanding
collection while consuming the iterable so despite appearances there's
no memory benefit to the generator, and list comprehensions are
translated into faster bytecode than general generators.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

qiskit/assembler/assemble_circuits.py

@@ -111,16 +111,16 @@ def _assemble_circuit(
     # their clbit_index, create a new register slot for every conditional gate
     # and add a bfunc to map the creg=val mask onto the gating register bit.
 
-    is_conditional_experiment = any(op.condition for (op, qargs, cargs) in circuit.data)
+    is_conditional_experiment = any(instruction.operation.condition for instruction in circuit.data)
     max_conditional_idx = 0
 
     instructions = []
     for op_context in circuit.data:
-        instruction = op_context[0].assemble()
+        instruction = op_context.operation.assemble()
 
         # Add register attributes to the instruction
-        qargs = op_context[1]
-        cargs = op_context[2]
+        qargs = op_context.qubits
+        cargs = op_context.clbits
         if qargs:
             instruction.qubits = [qubit_indices[qubit] for qubit in qargs]
         if cargs:

qiskit/circuit/__init__.py

@@ -177,6 +177,7 @@
    Clbit
    AncillaRegister
    AncillaQubit
+   CircuitInstruction
 
 Gates and Instructions
 ----------------------
@@ -238,6 +239,7 @@
 from .parameter import Parameter
 from .parametervector import ParameterVector
 from .parameterexpression import ParameterExpression
+from .quantumcircuitdata import CircuitInstruction
 from .equivalence import EquivalenceLibrary
 from .classicalfunction.types import Int1, Int2
 from .classicalfunction import classical_function, BooleanExpression

qiskit/circuit/add_control.py

@@ -121,19 +121,20 @@ def control(
             for index, bit in enumerate(bits)
         }
 
-        for gate, qargs, _ in definition.data:
+        for instruction in definition.data:
+            gate, qargs = instruction.operation, instruction.qubits
             if gate.name == "x":
                 controlled_circ.mct(q_control, q_target[bit_indices[qargs[0]]], q_ancillae)
             elif gate.name == "rx":
                 controlled_circ.mcrx(
-                    gate.definition.data[0][0].params[0],
+                    gate.definition.data[0].operation.params[0],
                     q_control,
                     q_target[bit_indices[qargs[0]]],
                     use_basis_gates=True,
                 )
             elif gate.name == "ry":
                 controlled_circ.mcry(
-                    gate.definition.data[0][0].params[0],
+                    gate.definition.data[0].operation.params[0],
                     q_control,
                     q_target[bit_indices[qargs[0]]],
                     q_ancillae,
@@ -142,7 +143,7 @@ def control(
                 )
             elif gate.name == "rz":
                 controlled_circ.mcrz(
-                    gate.definition.data[0][0].params[0],
+                    gate.definition.data[0].operation.params[0],
                     q_control,
                     q_target[bit_indices[qargs[0]]],
                     use_basis_gates=True,

qiskit/circuit/controlflow/builder.py

@@ -26,6 +26,7 @@
 from qiskit.circuit.classicalregister import Clbit, ClassicalRegister
 from qiskit.circuit.exceptions import CircuitError
 from qiskit.circuit.instruction import Instruction
+from qiskit.circuit.quantumcircuitdata import CircuitInstruction
 from qiskit.circuit.quantumregister import Qubit, QuantumRegister
 from qiskit.circuit.register import Register
 
@@ -238,7 +239,7 @@ def __init__(
                 here, and the documentation of :obj:`.InstructionSet`, which uses this same
                 callback.
         """
-        self.instructions: List[Tuple[Instruction, Tuple[Qubit, ...], Tuple[Clbit, ...]]] = []
+        self.instructions: List[CircuitInstruction] = []
         self.qubits = set(qubits)
         self.clbits = set(clbits)
         self.registers = set(registers)
@@ -260,12 +261,7 @@ def allow_jumps(self):
         """
         return self._allow_jumps
 
-    def append(
-        self,
-        operation: Instruction,
-        qubits: Iterable[Qubit],
-        clbits: Iterable[Clbit],
-    ) -> Instruction:
+    def append(self, instruction: CircuitInstruction) -> CircuitInstruction:
         """Add an instruction into the scope, keeping track of the qubits and clbits that have been
         used in total."""
         if not self._allow_jumps:
@@ -274,18 +270,16 @@ def append(
             from .continue_loop import ContinueLoopOp, ContinueLoopPlaceholder
 
             forbidden = (BreakLoopOp, BreakLoopPlaceholder, ContinueLoopOp, ContinueLoopPlaceholder)
-            if isinstance(operation, forbidden):
+            if isinstance(instruction.operation, forbidden):
                 raise CircuitError(
-                    f"The current builder scope cannot take a '{operation.name}'"
+                    f"The current builder scope cannot take a '{instruction.operation.name}'"
                     " because it is not in a loop."
                 )
 
-        qubits = tuple(qubits)
-        clbits = tuple(clbits)
-        self.instructions.append((operation, qubits, clbits))
-        self.qubits.update(qubits)
-        self.clbits.update(clbits)
-        return operation
+        self.instructions.append(instruction)
+        self.qubits.update(instruction.qubits)
+        self.clbits.update(instruction.clbits)
+        return instruction
 
     def request_classical_resource(self, specifier):
         """Resolve a single classical resource specifier into a concrete resource, raising an error
@@ -314,19 +308,18 @@ def request_classical_resource(self, specifier):
             self.add_register(resource)
         return resource
 
-    def peek(self) -> Tuple[Instruction, Tuple[Qubit, ...], Tuple[Clbit, ...]]:
+    def peek(self) -> CircuitInstruction:
         """Get the value of the most recent instruction tuple in this scope."""
         if not self.instructions:
             raise CircuitError("This scope contains no instructions.")
         return self.instructions[-1]
 
-    def pop(self) -> Tuple[Instruction, Tuple[Qubit, ...], Tuple[Clbit, ...]]:
-        """Get the value of the most recent instruction tuple in this scope, and remove it from this
+    def pop(self) -> CircuitInstruction:
+        """Get the value of the most recent instruction in this scope, and remove it from this
         object."""
         if not self.instructions:
             raise CircuitError("This scope contains no instructions.")
-        operation, qubits, clbits = self.instructions.pop()
-        return (operation, qubits, clbits)
+        return self.instructions.pop()
 
     def add_bits(self, bits: Iterable[Union[Qubit, Clbit]]):
         """Add extra bits to this scope that are not associated with any concrete instruction yet.
@@ -407,11 +400,14 @@ def build(
         # more later as needed.
         out = QuantumCircuit(list(self.qubits), list(self.clbits), *self.registers)
 
-        for operation, qubits, clbits in self.instructions:
-            if isinstance(operation, InstructionPlaceholder):
-                operation, resources = operation.concrete_instruction(all_qubits, all_clbits)
+        for instruction in self.instructions:
+            if isinstance(instruction.operation, InstructionPlaceholder):
+                operation, resources = instruction.operation.concrete_instruction(
+                    all_qubits, all_clbits
+                )
                 qubits = tuple(resources.qubits)
                 clbits = tuple(resources.clbits)
+                instruction = CircuitInstruction(operation, qubits, clbits)
                 # We want to avoid iterating over the tuples unnecessarily if there's no chance
                 # we'll need to add bits to the circuit.
                 if potential_qubits and qubits:
@@ -430,19 +426,15 @@ def build(
                         # a register is already present, so we use our own tracking.
                         self.add_register(register)
                         out.add_register(register)
-            if operation.condition is not None:
-                for register in condition_registers(operation.condition):
+            if instruction.operation.condition is not None:
+                for register in condition_registers(instruction.operation.condition):
                     if register not in self.registers:
                         self.add_register(register)
                         out.add_register(register)
             # We already did the broadcasting and checking when the first call to
             # QuantumCircuit.append happened (which the user wrote), and added the instruction into
             # this scope.  We just need to finish the job now.
-            #
-            # We have to convert to lists, because some parts of QuantumCircuit still expect
-            # exactly this type.
-            out._append(operation, list(qubits), list(clbits))
-
+            out._append(instruction)
         return out
 
     def copy(self) -> "ControlFlowBuilderBlock":

qiskit/circuit/controlflow/if_else.py

@@ -415,15 +415,13 @@ class ElseContext:
         Terra.
     """
 
-    __slots__ = ("_if_block", "_if_clbits", "_if_registers", "_if_context", "_if_qubits", "_used")
+    __slots__ = ("_if_instruction", "_if_registers", "_if_context", "_used")
 
     def __init__(self, if_context: IfContext):
         # We want to avoid doing any processing until we're actually used, because the `if` block
         # likely isn't finished yet, and we want to have as small a penalty a possible if you don't
         # use an `else` branch.
-        self._if_block = None
-        self._if_qubits = None
-        self._if_clbits = None
+        self._if_instruction = None
         self._if_registers = None
         self._if_context = if_context
         self._used = False
@@ -440,26 +438,22 @@ def __enter__(self):
             # I'm not even sure how you'd get this to trigger, but just in case...
             raise CircuitError("Cannot attach an 'else' to a broadcasted 'if' block.")
         appended = appended_instructions[0]
-        operation, _, _ = circuit._peek_previous_instruction_in_scope()
-        if appended is not operation:
+        instruction = circuit._peek_previous_instruction_in_scope()
+        if appended is not instruction:
             raise CircuitError(
                 "The 'if' block is not the most recent instruction in the circuit."
-                f" Expected to find: {appended!r}, but instead found: {operation!r}."
+                f" Expected to find: {appended!r}, but instead found: {instruction!r}."
             )
-        (
-            self._if_block,
-            self._if_qubits,
-            self._if_clbits,
-        ) = circuit._pop_previous_instruction_in_scope()
-        if isinstance(self._if_block, IfElseOp):
-            self._if_registers = set(self._if_block.blocks[0].cregs).union(
-                self._if_block.blocks[0].qregs
+        self._if_instruction = circuit._pop_previous_instruction_in_scope()
+        if isinstance(self._if_instruction.operation, IfElseOp):
+            self._if_registers = set(self._if_instruction.operation.blocks[0].cregs).union(
+                self._if_instruction.operation.blocks[0].qregs
             )
         else:
-            self._if_registers = self._if_block.registers()
+            self._if_registers = self._if_instruction.operation.registers()
         circuit._push_scope(
-            self._if_qubits,
-            self._if_clbits,
+            self._if_instruction.qubits,
+            self._if_instruction.clbits,
             registers=self._if_registers,
             allow_jumps=self._if_context.in_loop,
         )
@@ -472,32 +466,32 @@ def __exit__(self, exc_type, exc_val, exc_tb):
             # manager, assuming nothing else untoward happened to the circuit, but that's checked by
             # the __enter__ method.
             circuit._pop_scope()
-            circuit.append(self._if_block, self._if_qubits, self._if_clbits)
+            circuit._append(self._if_instruction)
             self._used = False
             return False
 
         false_block = circuit._pop_scope()
         # `if_block` is a placeholder if this context is in a loop, and a concrete instruction if it
         # is not.
-        if isinstance(self._if_block, IfElsePlaceholder):
-            if_block = self._if_block.with_false_block(false_block)
-            resources = if_block.placeholder_resources()
-            circuit.append(if_block, resources.qubits, resources.clbits)
+        if isinstance(self._if_instruction.operation, IfElsePlaceholder):
+            if_operation = self._if_instruction.operation.with_false_block(false_block)
+            resources = if_operation.placeholder_resources()
+            circuit.append(if_operation, resources.qubits, resources.clbits)
         else:
             # In this case, we need to update both true_body and false_body to have exactly the same
             # widths.  Passing extra resources to `ControlFlowBuilderBlock.build` doesn't _compel_
             # the resulting object to use them (because it tries to be minimal), so it's best to
             # pass it nothing extra (allows some fast path constructions), and add all necessary
             # bits onto the circuits at the end.
-            true_body = self._if_block.blocks[0]
+            true_body = self._if_instruction.operation.blocks[0]
             false_body = false_block.build(false_block.qubits, false_block.clbits)
             true_body, false_body = _unify_circuit_resources(true_body, false_body)
             circuit.append(
                 IfElseOp(
                     self._if_context.condition,
                     true_body,
                     false_body,
-                    label=self._if_block.label,
+                    label=self._if_instruction.operation.label,
                 ),
                 tuple(true_body.qubits),
                 tuple(true_body.clbits),
@@ -575,11 +569,11 @@ def _unify_circuit_resources_rebuild(  # pylint: disable=invalid-name  # (it's t
     clbits = list(set(true_body.clbits).union(false_body.clbits))
     # We use the inner `_append` method because everything is already resolved.
     true_out = QuantumCircuit(qubits, clbits, *true_body.qregs, *true_body.cregs)
-    for data in true_body.data:
-        true_out._append(*data)
+    for instruction in true_body.data:
+        true_out._append(instruction)
     false_out = QuantumCircuit(qubits, clbits, *false_body.qregs, *false_body.cregs)
-    for data in false_body.data:
-        false_out._append(*data)
+    for instruction in false_body.data:
+        false_out._append(instruction)
     return _unify_circuit_registers(true_out, false_out)
 
 

qiskit/circuit/duration.py

@@ -63,19 +63,20 @@ def convert_durations_to_dt(qc: QuantumCircuit, dt_in_sec: float, inplace=True):
     else:
         circ = qc.copy()
 
-    for inst, _, _ in circ.data:
-        if inst.unit == "dt" or inst.duration is None:
+    for instruction in circ.data:
+        operation = instruction.operation
+        if operation.unit == "dt" or operation.duration is None:
             continue
 
-        if not inst.unit.endswith("s"):
-            raise CircuitError(f"Invalid time unit: '{inst.unit}'")
+        if not operation.unit.endswith("s"):
+            raise CircuitError(f"Invalid time unit: '{operation.unit}'")
 
-        duration = inst.duration
-        if inst.unit != "s":
-            duration = apply_prefix(duration, inst.unit)
+        duration = operation.duration
+        if operation.unit != "s":
+            duration = apply_prefix(duration, operation.unit)
 
-        inst.duration = duration_in_dt(duration, dt_in_sec)
-        inst.unit = "dt"
+        operation.duration = duration_in_dt(duration, dt_in_sec)
+        operation.unit = "dt"
 
     if circ.duration is not None:
         circ.duration = duration_in_dt(circ.duration, dt_in_sec)

qiskit/circuit/equivalence.py

@@ -212,7 +212,8 @@ def _build_basis_graph(self):
                 decomp_basis = frozenset(
                     f"{name}/{num_qubits}"
                     for name, num_qubits in {
-                        (inst.name, inst.num_qubits) for inst, _, __ in decomp.data
+                        (instruction.operation.name, instruction.operation.num_qubits)
+                        for instruction in decomp.data
                     }
                 )
                 if basis not in node_map: