diff --git a/CHANGELOG.md b/CHANGELOG.md index b19335f..082ec5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,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 inaccurate `device_qubits` entry in `QasmModule.unroll()` docstring ([#349](https://github.com/qBraid/pyqasm/pull/349)) - Fixed `remove_idle_qubits()` and `reverse_qubit_order()` ignoring statements nested inside `box` and `if` blocks. Top-level operands were rewritten while nested ones kept their old indices, so the result silently addressed the wrong qubits — and when a nested index fell outside the shrunken register, the output was not a loadable program at all. Both passes now walk nested bodies, as do `has_measurements()` / `remove_measurements()` and `has_barriers()` / `remove_barriers()`; a box left empty by a removal is dropped, since pyqasm rejects a box with no statements. Two consequences of the same blind spot are fixed alongside: a qubit operated on only inside an `if` block no longer counts as idle, and `remove_idle_qubits()` no longer raises `AssertionError` on a program that mixes physical qubits with declared registers. ([#345](https://github.com/qBraid/pyqasm/pull/345)) - Fixed `unroll(consolidate_qubits=True)` raising `AttributeError: 'str' object has no attribute 'name'` for any gate applied to a physical qubit, e.g. `h $1;`. Consolidation assumed every gate operand was an `IndexedIdentifier`, but a physical qubit survives unrolling as `Identifier("$1")`. Physical qubits are absolute hardware indices belonging to no declared register, so they are now left as written — matching how `measure`, `reset` and `barrier` already treat them. ([#344](https://github.com/qBraid/pyqasm/pull/344)) diff --git a/src/pyqasm/visitor.py b/src/pyqasm/visitor.py index ea9f5f3..592a724 100644 --- a/src/pyqasm/visitor.py +++ b/src/pyqasm/visitor.py @@ -496,8 +496,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( @@ -505,6 +506,23 @@ def _qubit_register_consolidation( f"Total qubits '({total_qubits})' exceed device qubits '({self._module._device_qubits})'.", ) + # physical qubits are kept as written, so consolidating around them would emit + # two address spaces the output cannot relate (issue #353) + physical_qubits = sorted( + name for name, _ in self._module._qubit_depths if name.startswith("$") + ) + 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)})", + ) + # only physical qubits: nothing to consolidate, so do not declare an + # internal register nothing would reference + return unrolled_stmts + global_scope = self._scope_manager.get_global_scope() for var, val in global_scope.items(): if var == INTERNAL_QUBIT_REGISTER: diff --git a/tests/qasm3/test_device_qubits.py b/tests/qasm3/test_device_qubits.py index 7389633..3a97569 100644 --- a/tests/qasm3/test_device_qubits.py +++ b/tests/qasm3/test_device_qubits.py @@ -331,41 +331,66 @@ 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]; - cz $2, q[1]; - c = measure $2; + {operation} """ - expected_qasm = """OPENQASM 3.0; - qubit[5] __PYQASM_QUBITS__; + 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_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"; - bit[1] c; - h __PYQASM_QUBITS__[0]; - cz $2, __PYQASM_QUBITS__[1]; - c = measure $2; + 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]; """ 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) + result.unroll() 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 +398,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