Refactor output of OpenQASM 3 exporter to use fewer aliases

[jakelishman]

Summary

This removes spurious _loose_bit "registers" from the OpenQASM 3 output, and instead emits loose bits with individual bit and qubit declarations. Non-overlapping registers are emitted using regular bit[n] and qubit[n] definitions when possible, and we only resort to aliasing if we must to describe the structure.

This avoids introducing structure to the definitions that does not exist in the original program, making round-trips and interactions with other OQ3 consumers more straightforwards. It's better not to use advanced features that don't map to hardware particularly well when it's not necessary.

On the technical side, all bits are now properly tracked in the symbol table. Previously, there was a lot of code duplication, internal state tracking and magic inferences that attempted to "guess" how a qubit/clbit should be referred to. Instead, we just properly add them as variables to the symbol table, which also drastically reduces the number of objects that effectively reserve names that the user may not use.

Details and comments

As an example, given this code:

from qiskit import QuantumCircuit, QuantumRegister, ClassicalRegister, qasm3
from qiskit.circuit import Qubit, Clbit

qc = QuantumCircuit(
    QuantumRegister(3, "qmem"),
    [Qubit(), Qubit(), Qubit(), Clbit(), Clbit(), Clbit()],
    ClassicalRegister(3, "a"),
)
qc.h(0)
qc.cx(0, 1)
qc.cx(0, 2)
for i in range(3):
    qc.cx(i, i + 3)
qc.measure(range(6), range(6))

print(qasm3.dumps(qc))

the output before this PR was:

OPENQASM 3;
include "stdgates.inc";
bit[3] _loose_clbits;
bit[3] a;
qubit[6] _all_qubits;
let qmem = _all_qubits[0:2];
h qmem[0];
cx qmem[0], qmem[1];
cx qmem[0], qmem[2];
cx qmem[0], _all_qubits[3];
cx qmem[1], _all_qubits[4];
cx qmem[2], _all_qubits[5];
_loose_clbits[0] = measure qmem[0];
_loose_clbits[1] = measure qmem[1];
_loose_clbits[2] = measure qmem[2];
a[0] = measure _all_qubits[3];
a[1] = measure _all_qubits[4];
a[2] = measure _all_qubits[5];

and now it is the more natural

OPENQASM 3;
include "stdgates.inc";
bit _bit0;
bit _bit1;
bit _bit2;
bit[3] a;
qubit _qubit3;
qubit _qubit4;
qubit _qubit5;
qubit[3] qmem;
h qmem[0];
cx qmem[0], qmem[1];
cx qmem[0], qmem[2];
cx qmem[0], _qubit3;
cx qmem[1], _qubit4;
cx qmem[2], _qubit5;
_bit0 = measure qmem[0];
_bit1 = measure qmem[1];
_bit2 = measure qmem[2];
a[0] = measure _qubit3;
a[1] = measure _qubit4;
a[2] = measure _qubit5;
``

[qiskit-bot]

One or more of the the following people are requested to review this:

• @Qiskit/terra-core

[coveralls]

Pull Request Test Coverage Report for Build 5245029921

• 58 of 59 (98.31%) changed or added relevant lines in 3 files are covered.
• 8 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+0.03%) to 85.945%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/qasm3/exporter.py	49	50	98.0%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/expr.rs	1	93.76%
crates/qasm2/src/lex.rs	3	90.63%
crates/accelerate/src/sabre_swap/layer.rs	4	97.32%

Totals
Change from base Build 5244976406:	0.03%
Covered Lines:	71382
Relevant Lines:	83055

---

💛 - Coveralls

[ihincks]

Suggested change

	alias_classical_registers: bool = None,
	allow_aliasing: bool = None,
	alias_classical_registers: Optional[bool, None] = None,
	allow_aliasing: Optional[bool, None] = None,

[jakelishman]

I'm never super sure about this type of annotation. I mean, technically yes Optional[bool] (or Union[bool, None]) is correct for what the values are considered, but logically a caller should never pass None. It's just an implementation detail we use to track whether the option was manually specified during the switchover period. It'll stop accepting None once that period's over and just have the default False.

I can go either way on this - I don't know what type-hints users typically prefer.

[ihincks]

Please proceed with your preference

[ihincks]

Thanks @jakelishman , LGTM

[jlapeyre]

A minor point. Why is this line
https://github.com/Qiskit/qiskit-terra/blob/c365df87c62c58c378a242f57e4d563da42cea09/test/python/qasm3/test_export.py#L141
not written qubits = [Qubit() for _ in range(10)] ? Is it because the dummy variable _ is always set to None, suggesting that its value is irrelevant?

[jakelishman]

It's just a bit of an idiosyncrasy - there's no meaning. It so happens that [None]*n is one of the fastest iterators in CPython, despite the allocation. It's technically beaten by itertools.repeat(None, n), but for some reason I just tend to default to [None]*n.

[jlapeyre]

I'm having difficulty understanding the main point(s) of this PR. What is the problem with aliasing and "trying to guess how a qubit/clbit should be referred to" ?

Putting the summary at the top: I don't see that aliasing is essential if you want to collect loose bits as the existing code does (but I may be missing something). I do see that the PR may allow a more faithful round trip.

The example given in the opening comment doesn't make it clear (to me at least) that the change is better.
In this code

from qiskit import QuantumCircuit, QuantumRegister, ClassicalRegister, qasm3
from qiskit.circuit import Qubit, Clbit

n_qubits = 3
n_clbits = 3

use_register = False
if use_register:
    qubits = QuantumRegister(n_qubits, "qmem")
    clbits = ClassicalRegister(n_clbits, "a")
else:
    qubits = [Qubit() for _ in range(n_qubits)]
    clbits = [Clbit() for _ in range(n_clbits)]

qc = QuantumCircuit(qubits, clbits)

qc.h(0)
for i in range(1, n_qubits):
    qc.cx(0, i)

qc.measure(range(n_qubits), range(n_qubits)) # assume enough clbits
print(qasm3.dumps(qc))

what is the semantic difference when use_register is True vs False? Should it be reflected in the OQ3?
To me it looks cleaner in some cases, such as this example, to collect the bits in registers.

Is the following an instance of the unwanted aliasing?

qubit[3] _all_qubits;
let qmem = _all_qubits[0:2];

Can you avoid aliasing by outputting something like this ?

qubit[3] user_supplied_register_name_1;
qubit[3] _loose_qubits_1;
qubit[3] user_supplied_register_name_2;

One possible advantage that I can see of this PR is the following. If the OC3 representation doesn't collect "scalar" qubits into a register, then the round-trip representation in qiskit can be more faithful to the original. Maybe that's worth preserving.

EDIT: The following may be an example of the guess work or arbitrariness in referring to bits. If I have use a single list of qubits [Qubit() for _ in range(10)] then you could argue that it's natural to make this a register. But what about
[Qubit() for _ in range(10)], [special_qubit_1, another_qubit_for_a_different_purpose], QuantumRegister(3)] ?
I would still lose the names in going to OC3 and back, but at least I would not put unrelated qubits in a register if I did not do this in qiskit.

[jakelishman]

It's mostly about the OQ3 reflecting the structure of the Qiskit program more closely, which allows better round-tripping. That's originally why Ian had requested I do this; qiskit.qasm3.loads(qiskit.qasm2.dumps(qc)) == qc would typically be False if the circuit had loose bits, because the OQ3 dump would insert spurious structure.

The other problem is that a bit can be in more than one register. The original code always used aliasing so that there only needed to be a single path, but that quickly needed modifying when it became clear that hardware was never going to support the concept of aliasing classical registers, so the alias_classical_registers option was born. This PR simplifies a bunch of the magic that was added along with that option, moving the decision about "how should this bit be referenced?" into a single place, with the same logic working for both classical and quantum bits - previously, every place that referenced a bit did some magic guesswork to determine what OQ3 node should be output for a given bit.

To speak to the edit: whatever names you give your bits in Python space are irrelevant, because they're not stored in the QuantumCircuit. There's no possibility of round-tripping those in the current way Qiskit works. This is mostly just about making the Qiskit <-> OQ3 translation higher fidelity where there are simple ways to do that, and as a side-effect, it's a long overdue refactor to remove a lot of the magic names and tightly coupled logic between "definition" and "use" in the OQ3 exporter.

[jlapeyre]

It looks like a bit is referred to by its most recently created alias. Is this an implementation detail? Is it just an artifact of the implementation?

[jakelishman]

Mostly an implementation detail - I (vaguely) commented on the behaviour here: https://github.com/Qiskit/qiskit-terra/blob/6160ef6ea49f243052a3e5445706e0f8e2dfb6f8/qiskit/qasm3/exporter.py#L769-L775

[jlapeyre]

whatever names you give your bits in Python space are irrelevant, because they're not stored in the QuantumCircuit

I tried to say the same thing. The identifiers used in Python are not preserved as identifiers in OC3 (I don't know how to do this.) But whether the bit is in a Qiskit register or not can be preserved. I think this part of the structure you are talking about.

[jlapeyre]

An answer to my question above:

Is the following an instance of the unwanted aliasing?

qubit[3] _all_qubits;
let qmem = _all_qubits[0:2];

In fact you have simplified this for input QuantumRegister(3, "qmem"). I somehow missed this in my tests.

[jlapeyre]

LGTM as well.