Instruction should include qubits it acts on

[kevinsung]

What is the expected enhancement?

Qiskit currently lacks a class to represent the concept "a gate together with the qubits it acts on." This concept is useful because it is the basic unit a quantum circuit is composed of. For example, it can be the type of object appended to a circuit, so that QuantumCircuit.append can accept a single argument instead of multiple. It can also be used to simplify circuit construction in other ways; for example, circuit = QuantumCircuit(instructions) where instructions is an iterable of instructions (in fact it could be an "iterable tree" of instructions). After some discussion with @ajavadia we agree that the Instruction class could be updated to serve this purpose.

[jakelishman]

With due respect/apologies if I'm wrong to Ali, I think he might not be aware of some things that have already been discussed in the internal Terra meetings with the rest of the team on this front over the last month or so. This hits some of the internal development strands for the year (e.g. dynamic circuits, 1000q compilation, etc).

edit: to be clear, by "we" below I'm mostly referring to those in the fortnightly internal Terra meeting, and assigned to particular large-scale items on the roadmap (though wouldn't want to presume to speak for everyone), which is mostly people in the terra-core access group.

We are actually already in the process of adding the encapsulating class that you want. Matthew made a PR that staled (#7020), but that will be obsoleted and closed by what Erick and I have been working on (related to #7624), which we need to manage changing this internal 3-tuple to a new, 4-component type while maintaining API stability. It will essentially be a slightly fiddly data class, with some iteration tricks to make it look like the old 3-tuple when people use it as one. The tmp/instruction-data branch on my fork contains some of this, and the delay in getting it to a full PR is because I've been off work for a couple of weeks (but will be back on Monday).

We're trying to move more state out of Instruction, because at the moment it is trying to do too much, and the sheer number of this quite heavy class that need to be constructed causes memory issues, so we definitely don't want to move qargs into the existing Instruction. There's also some of uses of Instruction itself where this doesn't entirely make sense, for example the transpiler Target stores a single class instance of each to retrieve properties from, and we'd have to insert dummy data for qargs and cargs. The longer-term goal is to make Instruction subclasses as close as possible to stateless (including moving numeric parameters out of them), so things like XGate() become singleton objects.

For you usability points:

• We could update QuantumCircuit.append to also permit a single argument of type Tuple[Instruction, List[Qubit], List[Clbit]], which is the type you're describing - that is the internal object type of the quantum circuit data list (it's not always necessary need to add a wrapper class instead of tuple data in Python, and [WIP] Encapsulate instruction in args in "Instruction" class #7020 shows that doing so can have performance impacts). With the new encapsulated type, we've done exactly what you're after - I agree it's a good idea.

• QuantumCircuit(instructions): this alone wouldn't be a great constructor, because the ordering of the qubits that's created would be rather awkward for a user to work with afterwards. Perhaps you just meant a shorthand of adding an instructions keyword to the constructor? Since that's user data being input, we'd still have to check it all for consistency. If you can guarantee that your instructions iterable satisfies all the constraints and assumptions of QuantumCircuit, the preferred (and most performant) way to do this is already available:

  qc = QuantumCircuit(other.qubits, other.clbits)
  for instruction in instructions:
      qc._append(*instruction)

  QuantumCircuit._append is a semi-private method, and documented as such in the code (but not in the built docs unfortunately, due to me needing to update our Sphinx autosummary templates). In other words, we don't want to encourage non-advanced users to use it, but we will maintain its API stability. It's there for when your data is already verified correct for the circuit, such as building from a known-good source. We specifically do not do consistency checks between the data and the circuit (they're done by the public append, which calls this), so there's a fast path. The iterable-unpacking * will again be unnecessary once the encapulated class lands.

• the "iterable tree": I think you might mean a DAG? If so, you might find DAGCircuit along the lines of what you want, and there are already converters from it to QuantumCircuit (since we use it heavily in the transpiler). There's some on-going discussion about whether we'll just make this the type of QuantumCircuit.data (Kevin's been working with an external contributor on that, I think), but some of those plans may change as we discuss how we're going to handle the new classical control flow objects within the circuit data - Erick's finding some places where the current form might make the quantum parts of transpilation more difficult.

[kevinsung]

Thanks @jakelishman, it's good to know there is work being done on this front. In circuit = QuantumCircuit(instructions) I meant that instructions could have type Iterable[Instruction] where Instruction is the (proposed) class that represents "gate + qubits" which does not actually exist in Qiskit currently. By "iterable tree" I meant that it could in fact be a recursive type InstructionTree = Union[Instruction, Iterable[InstructionTree]] because this can be flattened into a list of instructions. The qubit ordering wouldn't be an issue if the user refers to the qubits directly rather than by index.

[jakelishman]

For sure - it's kind of as a side effect, but I think what we're planning hits just about everything you want already.

Yeah, that way of referring to qubit instances is my preferred way as well (and the most performant). I think at the moment we still need to be a little mindful of all the teaching docs we have out in the wild that promote integers as the primary method of indexing. Hopefully some of those problems will just solve themselves as we move to more dynamic circuits, where the output is no longer just one big flag classical register - just by default, indexing by instance will become more convenient once people aren't necessarily thinking about single registers.

[jakelishman]

Oh, I forgot to respond to the nested iterables bit sorry: I'm not 100% convinced that we should put a "flatten" operation in QuantumCircuit in favour of just having the user do it - it makes things a bit slower because we have to type-check every individual element of the iterable, which punishes people who don't want the nested structure. But I'm in favour of letting people pass the initial instruction data into the constructor in general, perhaps only as a keyword argument for now, and perhaps to promote it to be an allowed positional argument later.

(Right now it would be a bit messy internally to infer the qubits/clbits from only the encapsulated instruction iterable, because we'd have to duplicate some logic from append, and/or iterate through the instruction data more than once per element. Not impossible, but perhaps not worth it at first.)

[jakelishman]

Closing this as it's not the direction that's been taken for the data model, because we're separating the concept of "operation" (Instruction, in this case) from its runtime operands (qubits, clbits, some parameters, etc). This is to keep better composibility throughout the library, which will (hopefully) be helping our internal memory usage and avoid a lot of spurious copying in the library.