Move numeric parameters from Instruction to the data tuple

[jakelishman]

What should we add?

This is the first step in adding full classical data and instructions to QuantumCircuit. This is a partial step; it may well not be the only change to the data tuple that we make over the course of this, and it doesn't try to fix everything we dislike about Instruction yet, but it's a start.

I do recognise that what I suggest here is a breaking API change, but I think some degree of that is going to be unavoidable when we're making changes to basically the entirety of our core data structure and flow. I've tried to limit the damage.

Apologies that this is a bit long, I'm trying to write down all my thoughts about this step so we can get started while I'm out writing my thesis. @kdk is managing this issue while I'm out.

Why we chose this

We chose this because it should yield some tangible benefits immediately, without needing to consider the new type system yet:

• it lets the QASM exporters always access the parameterised definition of an instruction to emit correct code (e.g. QASM 2 and 3 exporters do not handle parameterised gates correctly #7335, QASM instruction not parameterized for two-qubit Pauli rotation gates #7172, and probably some more open issues)
• it will (hopefully) remove the need to deepcopy parameterised Instruction instances when binding parameters in circuits, in these lines: https://github.com/Qiskit/qiskit-terra/blob/13370342bca77000bf7321d8c581d5661b1a7fb7/qiskit/circuit/quantumcircuit.py#L1232-L1236
• it will make parameter binding more efficient and less error prone, because the binding will be lazier; the re-bind will naturally only happen at the top level by replacing the QuantumCircuit.data list entry, i.e. these (quite inefficient) lines should be made redundant: https://github.com/Qiskit/qiskit-terra/blob/13370342bca77000bf7321d8c581d5661b1a7fb7/qiskit/circuit/quantumcircuit.py#L2680-L2691

What to do

The aim is to move any params of existing Instructions that may be ParameterExpression into a fourth element of the (instruction, qargs, cargs) tuple. It might be convenient to do this in two steps:

1. convert the 3-tuple into a data class, so that everywhere needs to use named (dotted) access (i.e. instruction.operation, instruction.qargs and instruction.cargs). For now, it might need to be called something private (like _Instruction) because we haven't quite sorted out the migration path from the current Instruction. It would be good to make qargs and cargs into tuple, not list at this step - the immutability will help later.
2. add a fourth element to the class (called parameters by analogy to the current name, though strictly they'd be arguments not parameters!), and teach all the neceessary parts of Terra that to get the concrete definition or matrix form, they need to pass these in. This should also be a tuple, not a list.

Moving to named access is useful for updating the class in the future; it's easier to catch a name change with pylint than it is to catch the positional elements changing type or reordering. There may be a slight performance impact to this on the surface, but I think making this object mutable (but all its contents eventually immutable) will be much better performing in the long-run - copies of circuits and binding of parameters will become much cheaper, and getting to a point of internal immutability should let us massively reduce our memory footprint, which is great of itself but also has indirect performance advantages (even in Python).

In order to limit the immediate API breakage, it may be worth defining the Sequence interface on this new class as if it were still the old 3-tuple, so most tuple-like access on it will continue to work in user code. I would want to test suite to pass without this interface defined, though - we need to make sure Terra is fully updated.

Without actually trying it, it's hard to predict all the consequences, but here's some potential implementation considerations and pitfalls:

• Instruction/Operation will probably need to gain a new variable num_parameters for type checking.
• For compatibility right now, Instruction.definition should likely be a pointer to a parameterised QuantumCircuit, and it will be up to the caller to bind the parameters, when they need them to be bound. We need to ensure that the parameter order is fixed and correct.
• Similarly, Gate.inverse would ideally be defined to return a parameterised QuantumCircuit, which the caller will be responsible for binding when it needs to be bound.
• Instruction.__array__ (or Gate.__array__) will probably cease to exist, because the operation will not hold all the state necessary to construct a concrete array. That's deliberate - we don't want the operation (currently the Instruction instance) to hold the state any more.
• The internal ParameterTable structure in QuantumCircuit will need updating. A lot of Use ParameterReferences type for ParameterTable values #7408 is absolutely relevant here.
• The control-flow builder interface will need to start tracking Parameters as well. I think there are some subleties here (especially with IfElseContext and IfElseOp, which manage two circuit bodies), but I've not got so far as sketching out that process yet.

Limitations during this step

We don't want to do everything at once. There are lots of things that need more thought and design, and this change is already going to be very big, so we're trying to work incrementally. Several things in this step we know aren't perfect, but it will be easier to review the effects of changes and iterate the design if we go step-by-step.

• don't attempt to move things like ControlFlowOp's blocks, ControlledGate's ctrl_state or UnitaryGate's matrix out of the relevant Instruction. For now, the new parameters should be exactly the tuple of things that can validly be ParameterExpression.
• leave ForLoopOp's loop_parameter in the class, not in the new tuple - this is because the scoping of it may be internal to ForLoopOp.
• no need to fix the crazy Instruction.broadcast_arguments at this point - it's inconsistent and weird, and it can just stay that way
• there should be no need to mess with the Instruction/abstract Operation split - that's not fully in place yet, but the changes here should largely be orthogonal (except perhaps Operation needs the num_parameters variable).
• we will keep the split of cargs and parameters. This may change in the future once we have a stronger classical type system, but Clbit is always going to be special in some way (it's the only valid l-value of a measure), so it's good to keep it separate.
• we're not interested in solving the problem of dynamic qubit or clbit reference yet (i.e. we're not immediately trying to support the [not yet necessarily valid] OpenQASM 3 statement my_creg[i] = measure my_qreg[i]; in this step)

Some other things considered

Using a tuple or namedtuple for the elements of QuantumCircuit.data

It's hard to do ParameterTable and parameter binding efficiently if the elements are immutable - you would need a way to store a pointer into the QuantumCircuit.data structure that can be assigned to in constant time. For list, that's an integer index, except the index is kind of state-dependent; using that makes it impossible to insert or remove instructions in any efficient way at all - we'd have to loop through ParameterTable each time updating everything. We also need to think about the swap to a DAG data structure - there, it's going to be hard to store an "assignable" pointer.

These issues aren't so much of a problem if the qc.data element is mutable itself; we can use the same trick we currently do of storing it in the ParameterTable, and mutating it in-place to replace the parameters item. The current state of QuantumCircuit needs to deepcopy its Instructions to be safe. We wouldn't need to do this any more; the new lazier pattern means that we could safely simply shallow-copy the data element that's currently a tuple, since its components should be (or at least end up being) immutable.

Moving the entirety of Instruction.params into the tuple

We have rough desires to get the state out of Instruction entirely, but don't have a full plan for getting there. Separating out just the ParameterExpression bits first is something we know we'll need, because we need to start tracking data that gets fed into these slots for dynamic circuits, and there are the tangible benefits listed above to these. We will need to get these slots separately in the future in some form or another in order to make/bind QASM-ish gate my_gate(a, b, c) q1, q2 { ... } logical statements, as a, b and c are the only values that can be modified dynamically during the circuit.

If we move everything out immediately, we've mostly just shifted the issue, and we don't get most of the nice immediate benefits at the start of this issue.

[kevinsung]

If I understand correctly, this removes the "quantum gate" concept from Qiskit. For example, it would no longer be possible to construct an object that represents the gate Rx(0.1). If this is true, I worry that eliminating such a common concept would hurt Qiskit's ease of use, as well as ease of learning.

[jakelishman]

To some degree, yeah, but it's also kind of like saying that Qiskit doesn't have a concept of Rx(0.1 on qubit 0). We wouldn't say a classical RISC architecture doesn't have the instruction of add 1 because it implements a general add. The idea is to separate out operands from operations - rx is an operation, and "qubit 0" and 0.1 are the operands. The other things in the comment explain my reasons for why doing so is beneficial/necessary for supporting dynamic circuits.

For ease of use: we don't expect the internal data structures to be the preferred way for how users interact with our systems. There's no changes planned for the interface of QuantumCircuit.rx and friends right now. Frontend and backend are two separate concerns, and while they inform each other, they don't need to be completely identical - for example, the internal tree representation of control flow at the moment is quite tricky to work with for users, but we provide a builder interface that makes it drastically simpler. It got to the point when writing the tests for it, 95% of the time a failing test meant I'd got the tree structure wrong and the builders worked fine, but we still need to use a tree or some form of CFG internally.

[kevinsung]

There's no changes planned for the interface of QuantumCircuit.rx and friends right now.

This syntax does not work for gates which don't have a corresponding method on QuantumCircuit. This is an important use case, and one which is very commonly understood in terms of the gate concept (rather than the instruction/operation concept you refer to).

With this change, how would one obtain the matrix of a gate, say, Rx(0.1)?

[jakelishman]

The append interface is a bit more open to discussion, but long-term strategy is for an entirely new mechanism of building quantum circuits. It may well be the case that the circuit is constructed by a new DSL with syntax that looks like

with QuantumCircuit.build() as circuit:
  RX(0.1) | (0,)
  X | (1,)
  ...

but this is still a long way off - the internal discussions are very preliminary (and have been for a year). The first priority is to get the internal components of dynamic circuits working end-to-end, from Terra to backends, because it's no good having nice syntax if the system doesn't work. append will work with the new system (as in append(CircuitInstruction(RXGate, (qubit[0],), (), (0.1)))), it'll just be a little fiddlier at first.

In the internal-only structure, the current plan is to make Instruction.definition and a new Instruction.matrix (names not final) that take the numeric operands as parameters, so to get the matrix you'd do RXGate.matrix(0.1).

I appreciate that your applications think of gates as "gate + parameters" and that that's common, but equally common is the alternative of "RX is a gate and RX(0.1) is an application", especially at the transpilation and hardware layers. It's better for our internal data structures to be optimised for the latter, and we handle your higher-level view by syntactic sugar. The way you're thinking is a priority for us, it's just not the first priority.

[jakelishman]

I should also be clear that this won't happen overnight: we're committed to maintaining the API stability, so we'll be doing all we can to ensure that qc.append(RXGate(0.1), [0], []) continues to work for at least a few more Terra versions. It'll just get converted to something else internally.

[jakelishman]

This issue was obsoleted by the Rust-space work on CircuitData. We're not at a 100% clean split, but the internals of circuit-library standard gates now separate out the idea of the standard gate from its parameters, which was the heart of this issue.