diff --git a/CHANGELOG.md b/CHANGELOG.md index 575e9bc..b01220d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Types of changes: ### Removed ### Fixed +- Fixed `unroll(consolidate_qubits=True)` emitting two unrelated address spaces for a program mixing declared registers with physical qubits — a consolidated register plus as-written `$n` references. Such programs now raise a `ValidationError` naming the physical qubits, and a program using only physical qubits no longer receives an internal register declaration nothing references. ([#353](https://github.com/qBraid/pyqasm/issues/353)) - Fixed external and verbatim-box gates counting the depth of the decomposition they skipped: `unroll(external_gates=["crz"])` on a single `crz` reported `depth() == 12` while emitting one statement. An external gate now records its own depth, matching how a single-level external custom gate is already handled. ([#352](https://github.com/qBraid/pyqasm/issues/352)) - Fixed `has_measurements()` / `remove_measurements()` and `has_barriers()` / `remove_barriers()` missing occurrences inside `for` / `while` / `switch` bodies on a module that has not been unrolled — e.g. a measurement inside a `for` loop was invisible and `remove_measurements()` was a no-op. The statement walker now descends into loop and switch bodies. ([#354](https://github.com/qBraid/pyqasm/issues/354)) - Fixed unrolling of `rzz`/`rxx` in an OpenQASM 2 program emitting an invalid `gphase(...)` statement — syntax QASM 2 does not have — so the output was not a loadable QASM 2 program. Global phase is unobservable, so unroll-emitted phases are now dropped for a QASM 2 target; a user-written `gphase` is still rejected. A conditional left with no body by the drop is removed too, since QASM 2 has no form for an `if` without a `qop`. ([#351](https://github.com/qBraid/pyqasm/issues/351)) diff --git a/src/pyqasm/elements.py b/src/pyqasm/elements.py index b7dd303..3950da3 100644 --- a/src/pyqasm/elements.py +++ b/src/pyqasm/elements.py @@ -47,6 +47,26 @@ def is_internal_qubit_register(qubit_name: str) -> bool: ) +PHYSICAL_QUBIT_PREFIX = "$" +"""Prefix marking a physical qubit: an absolute hardware index that belongs to no +declared register.""" + + +def is_physical_qubit(qubit_name: str) -> bool: + """Check whether an identifier refers to a physical qubit ("$0", "$12"). + + The index must be a non-negative integer, so neither the bare prefix nor a name + such as "$foo" is a physical qubit. + + Args: + qubit_name (str): The identifier name to check. + + Returns: + bool: True if the identifier refers to a physical qubit. + """ + return qubit_name.startswith(PHYSICAL_QUBIT_PREFIX) and qubit_name[1:].isdigit() + + class InversionOp(Enum): """ Enum for specifying the inversion action of a gate. diff --git a/src/pyqasm/pulse/utils.py b/src/pyqasm/pulse/utils.py index 3469c0c..694aff9 100644 --- a/src/pyqasm/pulse/utils.py +++ b/src/pyqasm/pulse/utils.py @@ -22,7 +22,7 @@ import openqasm3.ast as qasm3_ast -from pyqasm.elements import INTERNAL_QUBIT_REGISTER +from pyqasm.elements import INTERNAL_QUBIT_REGISTER, is_physical_qubit from pyqasm.exceptions import raise_qasm3_error @@ -75,7 +75,7 @@ def process_qubits_for_openpulse_gate( # pylint: disable=too-many-arguments _qubit_set = set() for i, qubit in enumerate(operation.qubits): qubit_id = qubit.name.name if hasattr(qubit.name, "name") else qubit.name - if qubit_id.startswith("$") and qubit_id[1:].isdigit(): + if is_physical_qubit(qubit_id): if ( gate_op not in openpulse_qubit_map or qubit_id not in openpulse_qubit_map[gate_op] diff --git a/src/pyqasm/pulse/visitor.py b/src/pyqasm/pulse/visitor.py index 6507552..6128aa6 100644 --- a/src/pyqasm/pulse/visitor.py +++ b/src/pyqasm/pulse/visitor.py @@ -25,7 +25,7 @@ import openqasm3.ast as qasm3_ast -from pyqasm.elements import Capture, Frame, Variable, Waveform +from pyqasm.elements import Capture, Frame, Variable, Waveform, is_physical_qubit from pyqasm.exceptions import ( raise_qasm3_error, ) @@ -385,9 +385,7 @@ def _visit_barrier(self, barrier: qasm3_ast.QuantumBarrier) -> list[qasm3_ast.Qu """ if barrier.qubits: for qubit in barrier.qubits: - if isinstance(qubit, qasm3_ast.Identifier) and not ( - qubit.name.startswith("$") and qubit.name[1:].isdigit() - ): + if isinstance(qubit, qasm3_ast.Identifier) and not is_physical_qubit(qubit.name): frame = self._openpulse_scope_manager.get_from_global_scope(qubit.name) if frame is None: raise_qasm3_error( diff --git a/src/pyqasm/visitor.py b/src/pyqasm/visitor.py index 3b6fd73..0eca396 100644 --- a/src/pyqasm/visitor.py +++ b/src/pyqasm/visitor.py @@ -35,6 +35,7 @@ from pyqasm.analyzer import Qasm3Analyzer from pyqasm.elements import ( INTERNAL_QUBIT_REGISTER, + PHYSICAL_QUBIT_PREFIX, Capture, ClbitDepthNode, Context, @@ -44,6 +45,7 @@ Variable, Waveform, is_internal_qubit_register, + is_physical_qubit, ) from pyqasm.exceptions import ( BreakSignal, @@ -330,8 +332,9 @@ def _get_op_bits( else: reg_name = bit.name - if qubits and reg_name.startswith("$"): - # Physical qubit reference (e.g. $0, $1). + if qubits and reg_name.startswith(PHYSICAL_QUBIT_PREFIX): + # Physical qubit reference (e.g. $0, $1). Matched on the prefix alone so + # that a malformed index is reported rather than read as a register name. if not reg_name[1:].isdigit(): raise_qasm3_error( f"Invalid physical qubit identifier '{reg_name}': " @@ -502,8 +505,9 @@ def _qubit_register_consolidation( Raises: ValidationError: If the total number of qubits exceeds the available device qubits, - or if the reserved register '__PYQASM_QUBITS__' is already declared - in the original QASM program. + if the reserved register '__PYQASM_QUBITS__' is already declared in the + original QASM program, or if the program mixes declared registers with + physical qubits. """ if total_qubits > self._module._device_qubits: # type: ignore raise_qasm3_error( @@ -511,6 +515,8 @@ def _qubit_register_consolidation( f"Total qubits '({total_qubits})' exceed device qubits '({self._module._device_qubits})'.", ) + # checked before the physical-qubit exits below, so a program that declares the + # reserved name is told about that rather than about whatever it does next global_scope = self._scope_manager.get_global_scope() for var, val in global_scope.items(): if var == INTERNAL_QUBIT_REGISTER: @@ -519,6 +525,31 @@ def _qubit_register_consolidation( span=val.span, ) + # physical qubits are kept as written, so consolidating around them would emit + # two address spaces the output cannot relate (issue #353) + # _qubit_depths is keyed (name, index), so the hardware index is already + # there to order on -- no need to parse it back out of the name + physical_qubits = [ + name + for name, _ in sorted( + (key for key in self._module._qubit_depths if is_physical_qubit(key[0])), + key=lambda key: key[1], + ) + ] + if physical_qubits: + # presence, not capacity: a zero-sized declared register still declares + # a second address space + if self._global_qreg_size_map: + raise_qasm3_error( + "Cannot consolidate qubit registers: the program mixes declared " + f"registers with physical qubits ({', '.join(physical_qubits)}). " + "Unroll without 'consolidate_qubits=True', or rewrite the physical " + "qubits as operands of a declared register.", + ) + # only physical qubits: nothing to consolidate, so do not declare an + # internal register nothing would reference + return unrolled_stmts + pyqasm_reg_id = qasm3_ast.Identifier(INTERNAL_QUBIT_REGISTER) pyqasm_reg_size = qasm3_ast.IntegerLiteral(self._module._device_qubits) # type: ignore pyqasm_reg_stmt = qasm3_ast.QubitDeclaration(pyqasm_reg_id, pyqasm_reg_size) @@ -592,7 +623,7 @@ def _visit_measurement( # pylint: disable=too-many-locals,too-many-branches,too target = statement.target if isinstance(source, qasm3_ast.Identifier): is_pulse_gate = False - if source.name.startswith("$") and source.name[1:].isdigit(): + if is_physical_qubit(source.name): if self._openpulse_grammar_declared: # OpenPulse program: rename to the internal virtual register used by the # pulse visitor, and validate the index is in range. @@ -738,7 +769,7 @@ def _resolve_unindexed_reset_qubit(self, statement: qasm3_ast.QuantumReset) -> b return False qubit_name = statement.qubits.name - if qubit_name.startswith("$") and qubit_name[1:].isdigit(): + if is_physical_qubit(qubit_name): if self._openpulse_grammar_declared: # OpenPulse program: rename to the internal virtual register used by the # pulse visitor. @@ -870,7 +901,7 @@ def _visit_barrier( # pylint: disable=too-many-locals, too-many-branches valid_open_pulse_qubits = False for op_qubit in barrier.qubits: if isinstance(op_qubit, qasm3_ast.Identifier): - if op_qubit.name.startswith("$") and op_qubit.name[1:].isdigit(): + if is_physical_qubit(op_qubit.name): phys_idx = int(op_qubit.name[1:]) # In an OpenPulse program all physical qubits are declared up-front via # defcal; validate that the index is within the known range. @@ -3244,7 +3275,7 @@ def _visit_calibration_definition( if not isinstance(qubit, qasm3_ast.Identifier): continue name = qubit.name - if name.startswith("$") and name[1:].isdigit(): + if is_physical_qubit(name): _qubit_set.add(int(name[1:])) self._openpulse_qubit_map[statement.name.name].add(name) self._total_pulse_qubits = max(self._total_pulse_qubits, int(name[1:]) + 1) diff --git a/tests/qasm3/test_device_qubits.py b/tests/qasm3/test_device_qubits.py index 7389633..1a87cba 100644 --- a/tests/qasm3/test_device_qubits.py +++ b/tests/qasm3/test_device_qubits.py @@ -331,41 +331,120 @@ def test_incorrect_qubit_reg(qasm_code, error_message, error_span, caplog): assert error_span in caplog.text -def test_physical_qubits_are_not_consolidated(): - """Physical qubits are absolute hardware indices and belong to no declared register, - so consolidation must leave them alone instead of raising (see #343).""" - qasm = """OPENQASM 3.0; +@pytest.mark.parametrize( + "operation", + [ + "cz $2, q[1];", + "c = measure $2;", + "reset $2;", + "barrier $2;", + ], +) +def test_mixed_declared_and_physical_rejected_when_consolidating(operation): + """A program mixing declared registers with physical qubits would consolidate into + two address spaces the output cannot relate, so it is rejected (issue #353).""" + qasm = f"""OPENQASM 3.0; include "stdgates.inc"; qubit[2] q; bit c; h q[0]; + {operation} + """ + result = loads(qasm, device_qubits=5) + with pytest.raises( + ValidationError, match=r"mixes declared registers with physical qubits \(\$2\)" + ): + result.unroll(consolidate_qubits=True) + + +def test_mixed_error_lists_physical_qubits_in_numeric_order(): + """A lexicographic sort would report ($10, $2) once a device has ten or more + qubits, which reads as unordered when scanning for the offending references.""" + qasm = """OPENQASM 3.0; + include "stdgates.inc"; + qubit[1] q; + h q[0]; + h $2; + h $10; + """ + result = loads(qasm, device_qubits=16) + with pytest.raises( + ValidationError, match=r"mixes declared registers with physical qubits \(\$2, \$10\)" + ): + result.unroll(consolidate_qubits=True) + + +def test_mixed_error_names_the_way_out(): + """The message is the whole diagnostic here -- no statement node is available at + finalize time, so there is no line number to fall back on.""" + qasm = """OPENQASM 3.0; + include "stdgates.inc"; + qubit[1] q; + h q[0]; + h $2; + """ + result = loads(qasm, device_qubits=5) + with pytest.raises(ValidationError, match=r"Unroll without 'consolidate_qubits=True'"): + result.unroll(consolidate_qubits=True) + + +@pytest.mark.parametrize( + "declaration", ["int __PYQASM_QUBITS__ = 3;", "qubit[2] __PYQASM_QUBITS__;"] +) +def test_reserved_name_is_reported_before_physical_qubit_exits(declaration): + """Declaring the reserved name must raise even when the program also uses physical + qubits, which would otherwise return early or report the wrong problem.""" + qasm = f"""OPENQASM 3.0; + include "stdgates.inc"; + {declaration} + h $1; + """ + result = loads(qasm, device_qubits=5) + with pytest.raises(ValidationError, match=r"'__PYQASM_QUBITS__' is already defined"): + result.unroll(consolidate_qubits=True) + + +def test_zero_sized_register_still_counts_as_declared(): + """A zero-sized declared register is still a second address space (Argus P1).""" + qasm = """OPENQASM 3.0; + include "stdgates.inc"; + qubit[0] q; + h $1; + """ + result = loads(qasm) + with pytest.raises(ValidationError, match=r"mixes declared registers with physical qubits"): + result.unroll(consolidate_qubits=True) + + +def test_mixed_declared_and_physical_still_unrolls_without_consolidation(): + """The mixed-program rejection applies only under consolidate_qubits=True.""" + qasm = """OPENQASM 3.0; + include "stdgates.inc"; + qubit[2] q; + h q[0]; cz $2, q[1]; - c = measure $2; """ + result = loads(qasm, device_qubits=5) + result.unroll() expected_qasm = """OPENQASM 3.0; - qubit[5] __PYQASM_QUBITS__; include "stdgates.inc"; - bit[1] c; - h __PYQASM_QUBITS__[0]; - cz $2, __PYQASM_QUBITS__[1]; - c = measure $2; + qubit[2] q; + h q[0]; + cz $2, q[1]; """ - result = loads(qasm, device_qubits=5) - result.unroll(consolidate_qubits=True) check_unrolled_qasm(dumps(result), expected_qasm) - # two consolidated slots plus physical $2, which sizes the count to its own index + 1. - # neither number is the declared qubit[5], which comes from device_qubits (see #353) assert result.num_qubits == 3 def test_physical_qubits_only(): + """With nothing to consolidate, no internal register is declared: the program + keeps speaking the physical address space alone (issue #353).""" qasm = """OPENQASM 3.0; include "stdgates.inc"; h $1; cz $2, $1; """ expected_qasm = """OPENQASM 3.0; - qubit[5] __PYQASM_QUBITS__; include "stdgates.inc"; h $1; cz $2, $1; @@ -373,6 +452,17 @@ def test_physical_qubits_only(): result = loads(qasm, device_qubits=5) result.unroll(consolidate_qubits=True) check_unrolled_qasm(dumps(result), expected_qasm) - # nothing was consolidated, so the count comes entirely from the physical indices - # while the emitted declaration is sized by device_qubits (see #353) + # the count comes entirely from the physical indices assert result.num_qubits == 3 + + +def test_physical_qubits_only_without_device_qubits(): + """The unreferenced declaration is suppressed with or without device_qubits set.""" + qasm = """OPENQASM 3.0; + include "stdgates.inc"; + h $1; + """ + result = loads(qasm) + result.unroll(consolidate_qubits=True) + assert "__PYQASM_QUBITS__" not in dumps(result) + assert result.num_qubits == 2