Fix layout output with hardware qubits (#30)

This fixes the output of programs that involve hardware qubits to the
form more standard to how Qiskit represents physical circuits.  Now, the
output circuit will have as many qubits as the maximum hardware qubit
explicitly used, even if the lower qubits are not used.  The
`TranspileLayout` will be a mapping of the qubits to their own indices,
to indicate that there were no underlying virtual qubits, but to assist
with other tools in recognising that the circuit is defined on physical
qubits.

The previous handling of hardware qubits created a circuit with only
explicitly referenced qubits in the circuit, then attempted to convey
the information about the actual hardware indices via the
`TranspileLayout`.  Unfortunately, this was not in a form that was
readily understandable to other tools, and the model of the "physical"
circuit not directly having its bit indices correspond to the hardware
qubit index was at odds with Qiskit's usual model of physical circuits.

releasenotes/notes/fix-layout-holes-becf2dd6b849c066.yaml

@@ -0,0 +1,12 @@
+---
+fixes:
+  - |
+    Importing a circuit with physical qubits (for example ``$4``) will now create a
+    :class:`~qiskit.circuit.QuantumCircuit` that has as many qubits as implied by the maximum
+    physical-qubit index encountered.  For example, if the largest physical qubit encountered is
+    ``$4``, the output circuit will have five qubits.
+
+    Previously, the circuit would only have as many qubit objects as were explicitly used and the
+    :class:`~qiskit.transpiler.TranspileLayout` of the circuit would attempt to indicate the mapping,
+    but this was at odds with how Qiskit typically represents physical circuits, and the returned
+    layout was in a non-standard form.

src/qiskit_qasm3_import/converter.py

@@ -31,7 +31,7 @@
 from .data import Scope, Symbol
 from .exceptions import ConversionError, raise_from_node
 from .expression import ValueResolver, resolve_condition
-from .state import State, SymbolTables, LocalScope, GateScope, is_physical
+from .state import State, LocalScope, GateScope
 
 _QASM2_IDENTIFIER = re.compile(r"[a-z]\w*", flags=re.ASCII)
 
@@ -88,13 +88,6 @@ def _escape_qasm2(name: str) -> str:
     return name
 
 
-# A hardware-qubit symbol has the form '$' followed by digits.
-# The digits are the identifier used by backends.
-def hardware_qubit_map(symbol_table: SymbolTables):
-    "Return a `dict` mapping `Qubit` instances to `int`s representing physical qubit identifiers."
-    return {sym.data: int(sym.name[1:]) for sym in symbol_table.globals() if is_physical(sym)}
-
-
 class GateBuilder:
     def __init__(
         self, name: str, definition: QuantumCircuit, order: Optional[Sequence[Parameter]] = None
@@ -141,11 +134,14 @@ def convert(self, node: ast.Program, *, source: Optional[str] = None) -> Quantum
         stored in property thereof named `circuit`.
         """
 
-        state = self.visit(node, State(source))
-        hardware_qubits = hardware_qubit_map(state.symbol_table)
-        if len(hardware_qubits) > 0:
+        state: State = self.visit(node, State(source))
+        if state.addressing_mode.is_physical():
             # pylint: disable=protected-access
-            state.circuit._layout = TranspileLayout(Layout(hardware_qubits), hardware_qubits)
+            state.circuit._layout = TranspileLayout(
+                initial_layout=Layout.from_qubit_list(state.circuit.qubits),
+                input_qubit_mapping={bit: i for i, bit in enumerate(state.circuit.qubits)},
+                final_layout=None,
+            )
         return state
 
     def _raise_previously_defined(self, new: Symbol, old: Symbol, node: ast.QASMNode) -> NoReturn:

src/qiskit_qasm3_import/expression.py

@@ -97,15 +97,18 @@ def visit_Identifier(self, node: ast.Identifier):
         cxt = self._context
         if (symbol := cxt.symbol_table.get(node.name, node)) is not None:
             return symbol.data, symbol.type
-        if not state.is_physical(node.name):
+        if (index := state.physical_qubit_index(node.name)) is None:
             raise_from_node(node, f"Undefined symbol '{node.name}'.")
 
         cxt.addressing_mode.set_physical_mode(node)
-        bit = Qubit()
-        cxt.circuit.add_bits([bit])
-        symbol = Symbol(node.name, bit, types.HardwareQubit(), Scope.GLOBAL, node)
-        cxt.symbol_table.insert(symbol)
-        return symbol.data, symbol.type
+        num_qubits = cxt.circuit.num_qubits
+        new_identifiers = [ast.Identifier(name=f"${i}") for i in range(num_qubits, index + 1)]
+        new_bits = [Qubit() for _ in new_identifiers]
+        hardware_qubit = types.HardwareQubit()
+        for name, bit in zip(new_identifiers, new_bits):
+            cxt.symbol_table.insert(Symbol(name.name, bit, hardware_qubit, Scope.GLOBAL, None))
+        cxt.circuit.add_bits(new_bits)
+        return new_bits[-1], hardware_qubit
 
     def visit_IntegerLiteral(self, node: ast.IntegerLiteral):
         return node.value, types.Int(const=True)

src/qiskit_qasm3_import/state.py

@@ -29,14 +29,16 @@
 }
 
 
-_PHYSICAL_QUBIT_RE = re.compile(r"\$\d+")
+_PHYSICAL_QUBIT_RE = re.compile(r"\$(?P<index>\d+)")
 
 
-def is_physical(name: Union[str, Symbol]):
-    "Return true if name is a valid identifier for a physical qubit."
+def physical_qubit_index(name: Union[str, Symbol]) -> Optional[int]:
+    """If this name is a physical qubit, return its integer index.  If not, return ``None``."""
     if isinstance(name, Symbol):
         name = name.name
-    return re.match(_PHYSICAL_QUBIT_RE, name) is not None
+    if match := _PHYSICAL_QUBIT_RE.fullmatch(name):
+        return int(match["index"])
+    return None
 
 
 class AddressingMode:
@@ -132,7 +134,7 @@ def __contains__(self, name: str):
 
     def get(self, name: str, node=None):
         top_scope = self[len(self) - 1].scope
-        if top_scope is Scope.GATE and is_physical(name):
+        if top_scope is Scope.GATE and physical_qubit_index(name) is not None:
             raise_from_node(
                 node,
                 f"Illegal qubit reference '{name}'. References to hardware "

tests/test_convert.py

@@ -11,6 +11,7 @@
     Qubit,
 )
 from qiskit.quantum_info import Operator
+from qiskit.transpiler import TranspileLayout, Layout
 
 from qiskit_qasm3_import import parse, ConversionError
 
@@ -172,13 +173,79 @@ def test_undeclared_physical_qubit():
         reset $1;
     """
     qc = parse(source)
-    expected = QuantumCircuit([Qubit()])
-    expected.reset(0)
+    expected = QuantumCircuit([Qubit(), Qubit()])
+    expected.reset(1)
     assert len(qc.qubits) == len(expected.qubits)
     assert qc.qregs == expected.qregs
     assert qc == expected
 
 
+def test_undeclared_physical_qubits_with_gaps():
+    """We should output a circuit that has as many qubits as the highest physical qubit used, since
+    Qiskit only represents physical qubits by integer indices."""
+    source = """
+        include "stdgates.inc";
+        bit[2] c;
+        h $3;
+        cx $5, $3;
+        c[0] = measure $3;
+        c[1] = measure $5;
+    """
+    qc = parse(source)
+    expected = QuantumCircuit([Qubit() for _ in range(6)], ClassicalRegister(2, "c"))
+    expected.h(3)
+    expected.cx(5, 3)
+    expected.measure([3, 5], [0, 1])
+    assert qc == expected
+
+    # Note we have to use the 'Qubit' instances of the parsed circuit when comparing layouts, since
+    # that's outside the context of the full comparison.
+    expected_layout = TranspileLayout(
+        initial_layout=Layout.from_qubit_list(qc.qubits),
+        input_qubit_mapping={bit: i for i, bit in enumerate(qc.qubits)},
+        final_layout=None,
+    )
+    assert qc.layout == expected_layout
+
+
+def test_undeclared_physical_qubits_in_control_flow():
+    source = """
+        include "stdgates.inc";
+        bit[2] c;
+        if (c[0]) {
+            h $7;
+        }
+        while (c == 0) {
+            while (!c[0]) {
+                h $3;
+                cx $3, $9;
+                c[0] = measure $3;
+                c[1] = measure $9;
+            }
+        }
+        h $9;
+    """
+    qc = parse(source)
+    cr = ClassicalRegister(2, "c")
+    expected = QuantumCircuit([Qubit() for _ in range(10)], cr)
+    with expected.if_test((cr[0], True)):
+        expected.h(7)
+    with expected.while_loop((cr, 0)):
+        with expected.while_loop((cr[0], False)):
+            expected.h(3)
+            expected.cx(3, 9)
+            expected.measure([3, 9], [0, 1])
+    expected.h(9)
+    assert qc == expected
+
+    expected_layout = TranspileLayout(
+        initial_layout=Layout.from_qubit_list(qc.qubits),
+        input_qubit_mapping={bit: i for i, bit in enumerate(qc.qubits)},
+        final_layout=None,
+    )
+    assert qc.layout == expected_layout
+
+
 def test_physical_qubit_stdgates():
     source = """
         include 'stdgates.inc';
@@ -1098,91 +1165,3 @@ def test_hardware_mode_and_user_gates():
     expected = QuantumCircuit([Qubit()])
     expected.u(0, 4.5, 0, 0)
     assert qc.data[1].operation.definition == expected
-
-
-# pylint: disable=protected-access
-def test_layout_for_hardware_qubits():
-    source = """
-        reset $99;
-    """
-    qc = parse(source)
-    expected = QuantumCircuit([Qubit()])
-    expected.reset(0)
-    assert qc == expected
-    assert qc._layout.input_qubit_mapping == {qc.qubits[0]: 99}
-
-
-def test_hardware_qubit_local_scope():
-    source = """
-    include "stdgates.inc";
-
-    bit[2] mid;
-
-    reset $101;
-    reset $100;
-    while (mid == "00") {
-        h $100;
-        h $101;
-        mid[0] = measure $100;
-        mid[1] = measure $101;
-     }
-    """
-    qc = parse(source)
-    assert qc._layout.input_qubit_mapping == dict(zip(qc.qubits, [101, 100]))
-
-
-def test_hardware_qubit_nested_scope():
-    source = """
-    include "stdgates.inc";
-
-    bit[2] mid;
-
-    reset $100;
-    reset $101;
-
-    if (mid[0]) {
-        while (mid == "00") {
-            h $100;
-            h $101;
-            mid[0] = measure $100;
-            mid[1] = measure $101;
-         }
-    }
-    """
-    qc = parse(source)
-    assert qc._layout.input_qubit_mapping == dict(zip(qc.qubits, [100, 101]))
-
-
-def test_hardware_qubit_first_seen_in_local_scope():
-    source = """
-    include "stdgates.inc";
-
-    bit[2] mid;
-
-    h $102;
-
-    while (mid == "00") {
-        h $0;
-        h $101;
-        mid[0] = measure $101;
-        mid[1] = measure $0;
-     }
-    """
-    qc = parse(source)
-    assert qc._layout.input_qubit_mapping == dict(zip(qc.qubits, [102, 0, 101]))
-
-
-def test_use_hardware_qubit_first_seen_in_local_scope():
-    source = """
-    include "stdgates.inc";
-
-    bit[1] mid;
-
-    while (mid == "0") {
-        h $100;
-        mid[0] = measure $100;
-     }
-    x $100;
-    """
-    qc = parse(source)
-    assert qc._layout.input_qubit_mapping == dict(zip(qc.qubits, [100]))