From 34693e1fd768b3c1a315266c04dcd1f38d31b5bf Mon Sep 17 00:00:00 2001 From: Ryan Hill Date: Fri, 7 Aug 2026 08:08:26 -0500 Subject: [PATCH 1/4] fix: drop unroll-emitted global phase for QASM 2 targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rzz/rxx decompose to a body containing a QuantumPhase node, which Qasm2Module emitted as gphase(...) — syntax OpenQASM 2 does not have, so the unrolled output was not a loadable QASM 2 program. Global phase is unobservable, so Qasm2Module.accept() now strips QuantumPhase statements (descending into conditional bodies) from the unrolled AST. User-written gphase in QASM 2 source is still rejected. Fixes #351 --- CHANGELOG.md | 1 + src/pyqasm/modules/qasm2.py | 24 ++++-- tests/qasm2/test_conditional_body.py | 10 +-- tests/qasm2/test_operations.py | 107 +++++++++++++++++++++++++++ 4 files changed, 131 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bcc32f06..c0b0952a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Types of changes: ### Removed ### Fixed +- Fixed `rzz`/`rxx` in an OpenQASM 2 program unrolling to a `gphase(...)` statement, which QASM 2 has no syntax for, 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. ([#351](https://github.com/qBraid/pyqasm/issues/351)) - 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)) - Fixed `unroll()` and `rebase()` emitting statements that share operand AST nodes: gate decompositions passed the same `IndexedIdentifier` objects into every statement they emitted, so transformations that rewrite qubit indices in place mutated a shared node once per referencing statement. This crashed `reverse_qubit_order()` (`KeyError: -1`) and `remove_idle_qubits()` (`KeyError`, [#331](https://github.com/qBraid/pyqasm/issues/331)) on any decomposed gate (e.g. `crz`) whenever the remap was not the identity. Statement constructors in `maps/gates.py` and `Decomposer` now copy their qubit operands so every emitted statement owns its nodes. ([#333](https://github.com/qBraid/pyqasm/issues/333)) diff --git a/src/pyqasm/modules/qasm2.py b/src/pyqasm/modules/qasm2.py index bba821ed..c7246889 100644 --- a/src/pyqasm/modules/qasm2.py +++ b/src/pyqasm/modules/qasm2.py @@ -96,12 +96,11 @@ def _filter_branch_body(self, statement: qasm3_ast.BranchingStatement): self._filter_branch_body(inner_stmt) continue if isinstance(inner_stmt, qasm3_ast.QuantumPhase): - # not something the user wrote: rzz/rxx decompose to a global phase, so this - # is only reachable by re-filtering an already-unrolled body (see issue #351) + # unroll-emitted phases are dropped in accept() (issue #351), so only a + # user-written gphase reaches this raise_qasm3_error( "Global phase is not representable in QASM 2.0, so it cannot appear in " - "a conditional body; it is introduced by unrolling gates such as 'rzz' " - "and 'rxx'", + "a conditional body", error_node=inner_stmt, span=inner_stmt.span, ) @@ -149,6 +148,21 @@ def to_qasm3(self, as_str: bool = False) -> str | Qasm3Module: qasm_program.version = "3.0" return dumps(qasm_program) if as_str else Qasm3Module(self._name, qasm_program) + def _drop_global_phase(self, statements): + """Remove QuantumPhase statements the unroller emitted (e.g. from the rzz/rxx + decompositions), descending into conditional bodies. OpenQASM 2 has no + global-phase syntax, and a global phase is unobservable, so dropping it is + semantically safe (issue #351).""" + filtered = [] + for stmt in statements: + if isinstance(stmt, qasm3_ast.QuantumPhase): + continue + if isinstance(stmt, qasm3_ast.BranchingStatement): + stmt.if_block = self._drop_global_phase(stmt.if_block) + stmt.else_block = self._drop_global_phase(stmt.else_block) + filtered.append(stmt) + return filtered + def accept(self, visitor): """Accept a visitor for the module @@ -159,4 +173,4 @@ def accept(self, visitor): unrolled_stmt_list = visitor.visit_basic_block(self._statements) final_stmt_list = visitor.finalize(unrolled_stmt_list) - self.unrolled_ast.statements = final_stmt_list + self.unrolled_ast.statements = self._drop_global_phase(final_stmt_list) diff --git a/tests/qasm2/test_conditional_body.py b/tests/qasm2/test_conditional_body.py index b093a2d4..98f1fe54 100644 --- a/tests/qasm2/test_conditional_body.py +++ b/tests/qasm2/test_conditional_body.py @@ -64,15 +64,13 @@ def test_conditional_non_qop_rejected(operation, keyword): module.validate() -def test_conditional_global_phase_reports_global_phase(): - """Test that the QuantumPhase unrolling introduces for rzz/rxx is reported as global - phase rather than as an AST class name. Reachable only by re-filtering an already - unrolled body, which remove_idle_qubits/reverse_qubit_order do (issue #351).""" +def test_conditional_rzz_survives_refiltering(): + """Test that transformations which re-filter an already unrolled body no longer + trip over the rzz global phase: it is dropped for a QASM 2 target (issue #351)""" module = loads(QASM2_PREAMBLE + "if(m==1) rzz(0.3) q[0], q[1];\n") module.unroll() module.reverse_qubit_order() - with pytest.raises(ValidationError, match="Global phase is not representable in QASM 2.0"): - module.remove_idle_qubits() + module.remove_idle_qubits() @pytest.mark.parametrize( diff --git a/tests/qasm2/test_operations.py b/tests/qasm2/test_operations.py index 8c14ec96..2a99a6ec 100644 --- a/tests/qasm2/test_operations.py +++ b/tests/qasm2/test_operations.py @@ -67,6 +67,113 @@ def test_whitelisted_ops(): check_unrolled_qasm(dumps(result), expected_qasm) +def test_rzz_unrolls_without_gphase(): + """Test that the global phase from the rzz decomposition is dropped for a QASM 2 + target, which has no global-phase syntax (issue #351)""" + qasm2_string = """ + OPENQASM 2.0; + include 'qelib1.inc'; + qreg q[2]; + rzz(0.3) q[0], q[1]; + """ + + expected_qasm = """ + OPENQASM 2.0; + include 'qelib1.inc'; + qreg q[2]; + cx q[0], q[1]; + rz(0.3) q[1]; + rx(1.5707963267948966) q[1]; + rz(3.141592653589793) q[1]; + rx(1.5707963267948966) q[1]; + rz(3.141592653589793) q[1]; + cx q[0], q[1]; + """ + + result = loads(qasm2_string) + result.unroll() + check_unrolled_qasm(dumps(result), expected_qasm) + + +def test_rxx_unrolls_without_gphase(): + """Test that the global phase from the rxx decomposition is dropped for a QASM 2 + target (issue #351)""" + qasm2_string = """ + OPENQASM 2.0; + include 'qelib1.inc'; + qreg q[2]; + rxx(0.3) q[0], q[1]; + """ + + expected_qasm = """ + OPENQASM 2.0; + include 'qelib1.inc'; + qreg q[2]; + h q[0]; + h q[1]; + cx q[0], q[1]; + rz(0.3) q[1]; + cx q[0], q[1]; + h q[1]; + h q[0]; + """ + + result = loads(qasm2_string) + result.unroll() + check_unrolled_qasm(dumps(result), expected_qasm) + + +def test_conditional_rzz_unrolls_without_gphase(): + """Test that a conditional rzz body carries no gphase statement either (issue #351)""" + qasm2_string = """ + OPENQASM 2.0; + include 'qelib1.inc'; + qreg q[2]; + creg m[1]; + measure q[0] -> m[0]; + if(m==1) rzz(0.3) q[0], q[1]; + """ + + result = loads(qasm2_string) + result.unroll() + unrolled = dumps(result) + assert "gphase" not in unrolled + + # the unrolled output must be a loadable QASM 2 program + loads(unrolled).validate() + + +def test_unrolled_qasm2_round_trips(): + """Test that unrolled rzz output loads and re-unrolls cleanly: no gphase means the + second filtering pass has nothing to reject (issue #351)""" + qasm2_string = """ + OPENQASM 2.0; + include 'qelib1.inc'; + qreg q[2]; + rzz(0.3) q[0], q[1]; + """ + + result = loads(qasm2_string) + result.unroll() + round_tripped = loads(dumps(result)) + round_tripped.unroll() + check_unrolled_qasm(dumps(round_tripped), dumps(result)) + + +def test_user_written_gphase_rejected(): + """Test that a gphase statement written in QASM 2 source is still rejected -- + OpenQASM 2 has no global-phase syntax, so only unroller-introduced phases are dropped""" + qasm2_string = """ + OPENQASM 2.0; + include 'qelib1.inc'; + qreg q[2]; + gphase(0.3); + """ + + with pytest.raises(ValidationError): + loads(qasm2_string).validate() + + def test_subroutine_blacklist(): # subroutines From d3c9e0e96cbb37b50effe587a702a924ef430df0 Mon Sep 17 00:00:00 2001 From: Ryan Hill Date: Fri, 7 Aug 2026 10:35:03 -0500 Subject: [PATCH 2/4] docs: clarify gphase changelog wording --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0b0952a..1278444e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ Types of changes: ### Removed ### Fixed -- Fixed `rzz`/`rxx` in an OpenQASM 2 program unrolling to a `gphase(...)` statement, which QASM 2 has no syntax for, 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. ([#351](https://github.com/qBraid/pyqasm/issues/351)) +- 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. ([#351](https://github.com/qBraid/pyqasm/issues/351)) - 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)) - Fixed `unroll()` and `rebase()` emitting statements that share operand AST nodes: gate decompositions passed the same `IndexedIdentifier` objects into every statement they emitted, so transformations that rewrite qubit indices in place mutated a shared node once per referencing statement. This crashed `reverse_qubit_order()` (`KeyError: -1`) and `remove_idle_qubits()` (`KeyError`, [#331](https://github.com/qBraid/pyqasm/issues/331)) on any decomposed gate (e.g. `crz`) whenever the remap was not the identity. Statement constructors in `maps/gates.py` and `Decomposer` now copy their qubit operands so every emitted statement owns its nodes. ([#333](https://github.com/qBraid/pyqasm/issues/333)) From 85620fe25b1832546eda563600239589e54d9e6a Mon Sep 17 00:00:00 2001 From: Ryan Hill Date: Thu, 13 Aug 2026 15:11:18 -0500 Subject: [PATCH 3/4] fix: drop a conditional emptied by the global-phase drop, guard modified phase Address review on #358: - A gate whose body is only gphase left 'if (m[0] == true) {}' behind -- an if with no qop, which QASM 2 has no form for. Turning main's loud rejection into silently malformed output was the worse trade, so the branch is now dropped when both blocks come back empty. - Assert the invariant the drop relies on rather than leaning on it: a modified (controlled) phase is relative and observable, and the visitor rewrites those to 'p' gates, so none should reach here. - Restore coverage for the conditional-body gphase diagnostic this PR reworded, and pin the emptied-conditional case. - Narrow the round-trip comment in test_operations.py: validate() proves pyqasm re-reads its output, not that qiskit.qasm2 accepts it (it does not -- the conditional still prints in QASM 3 syntax, which is #338). --- CHANGELOG.md | 2 +- src/pyqasm/modules/qasm2.py | 12 ++++++++++++ tests/qasm2/test_conditional_body.py | 19 ++++++++++++++++++- tests/qasm2/test_operations.py | 4 +++- 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d54bfc4..4bf578e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ Types of changes: ### Removed ### Fixed -- 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. ([#351](https://github.com/qBraid/pyqasm/issues/351)) +- 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)) - 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/modules/qasm2.py b/src/pyqasm/modules/qasm2.py index c7246889..7cfd761d 100644 --- a/src/pyqasm/modules/qasm2.py +++ b/src/pyqasm/modules/qasm2.py @@ -156,10 +156,22 @@ def _drop_global_phase(self, statements): filtered = [] for stmt in statements: if isinstance(stmt, qasm3_ast.QuantumPhase): + # a controlled phase is relative, not global, and is observable; the + # visitor rewrites those to 'p' gates, so none should reach here + if stmt.modifiers: + raise_qasm3_error( + "Modified global phase cannot be dropped for a QASM 2 target", + error_node=stmt, + span=stmt.span, + ) continue if isinstance(stmt, qasm3_ast.BranchingStatement): stmt.if_block = self._drop_global_phase(stmt.if_block) stmt.else_block = self._drop_global_phase(stmt.else_block) + if not stmt.if_block and not stmt.else_block: + # the body was nothing but global phase, and QASM 2 has no + # form for a conditional without a qop + continue filtered.append(stmt) return filtered diff --git a/tests/qasm2/test_conditional_body.py b/tests/qasm2/test_conditional_body.py index 98f1fe54..64479dde 100644 --- a/tests/qasm2/test_conditional_body.py +++ b/tests/qasm2/test_conditional_body.py @@ -19,7 +19,7 @@ import pytest -from pyqasm.entrypoint import loads +from pyqasm.entrypoint import dumps, loads from pyqasm.exceptions import ValidationError QASM2_PREAMBLE = """OPENQASM 2.0; @@ -73,6 +73,23 @@ def test_conditional_rzz_survives_refiltering(): module.remove_idle_qubits() +def test_conditional_user_gphase_rejected(): + """Test that a gphase the user wrote in a conditional body is still rejected. Only + unroll-emitted phases are dropped; this branch keeps its own diagnostic (issue #351)""" + module = loads(QASM2_PREAMBLE + "if(m==1) gphase(0.3);\n") + with pytest.raises(ValidationError, match="Global phase is not representable in QASM 2.0"): + module.validate() + + +def test_conditional_emptied_by_phase_drop_is_removed(): + """Test that a conditional whose body was nothing but global phase is dropped rather + than emitted bodiless: QASM 2 has no form for an 'if' without a qop (issue #351)""" + module = loads(QASM2_PREAMBLE + "gate ph(t) a { gphase(t); }\nif(m==1) ph(0.3) q[1];\n") + module.unroll() + assert "if" not in dumps(module) + loads(dumps(module)).validate() + + @pytest.mark.parametrize( "operation", ["x q[1];", "reset q[1];", "measure q[1] -> c[0];", "cx q[0], q[1];"] ) diff --git a/tests/qasm2/test_operations.py b/tests/qasm2/test_operations.py index 2a99a6ec..bd36464d 100644 --- a/tests/qasm2/test_operations.py +++ b/tests/qasm2/test_operations.py @@ -139,7 +139,9 @@ def test_conditional_rzz_unrolls_without_gphase(): unrolled = dumps(result) assert "gphase" not in unrolled - # the unrolled output must be a loadable QASM 2 program + # the unrolled output must still re-load in pyqasm. It is not yet accepted by a + # strict QASM 2 parser: the conditional still prints as `if (m[0] == true) { ... }`, + # which is QASM 3 syntax. That half is #338's territory, not this PR's. loads(unrolled).validate() From 1f7db551c7200adf3f00e632ceeffe5086b93c96 Mon Sep 17 00:00:00 2001 From: TheGupta2012 Date: Mon, 17 Aug 2026 13:19:34 +0530 Subject: [PATCH 4/4] refactor: route final statement transformations through module.finalize The global-phase drop was applied inline in Qasm2Module.accept, leaving the two accept implementations to differ in their last statement. QasmModule gains a finalize hook that returns the statement list unchanged. Qasm2Module overrides it to drop the global phase, and both accept bodies now close the same way. A dialect that cannot express something the unroller emits has one named place to say so. Co-Authored-By: Claude Opus 5 (1M context) --- src/pyqasm/modules/base.py | 16 ++++++++++++++++ src/pyqasm/modules/qasm2.py | 15 ++++++++++++--- src/pyqasm/modules/qasm3.py | 2 +- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/pyqasm/modules/base.py b/src/pyqasm/modules/base.py index d91362d1..52bb2326 100644 --- a/src/pyqasm/modules/base.py +++ b/src/pyqasm/modules/base.py @@ -813,6 +813,22 @@ def copy(self) -> QasmModule: """Return a deep copy of the module.""" return deepcopy(self) + def finalize(self, statements: list[qasm3_ast.Statement]) -> list[qasm3_ast.Statement]: + """Apply dialect-specific transformations to the finalized statement list. + + Runs after the visitor has unrolled and finalized the program, on the + statements about to become the unrolled AST. The base implementation + returns them unchanged; a subclass overrides this when its dialect + cannot express something the unroller emits. + + Args: + statements (list[Statement]): The finalized statements. + + Returns: + list[Statement]: The statements to store as the unrolled AST. + """ + return statements + @abstractmethod def _qasm_ast_to_str(self, qasm_ast: Program) -> str: """Convert the qasm AST to a string.""" diff --git a/src/pyqasm/modules/qasm2.py b/src/pyqasm/modules/qasm2.py index a78c4b3b..fd176d9d 100644 --- a/src/pyqasm/modules/qasm2.py +++ b/src/pyqasm/modules/qasm2.py @@ -147,6 +147,17 @@ def to_qasm3(self, as_str: bool = False) -> str | Qasm3Module: qasm_program.version = "3.0" return dumps(qasm_program) if as_str else Qasm3Module(self._name, qasm_program) + def finalize(self, statements: list[qasm3_ast.Statement]) -> list[qasm3_ast.Statement]: + """Apply the QASM 2 transformations the finalized statement list needs. + + Args: + statements (list[Statement]): The finalized statements. + + Returns: + list[Statement]: The statements to store as the unrolled AST. + """ + return self._drop_global_phase(statements) + def _drop_global_phase( self, statements: list[qasm3_ast.Statement] ) -> list[qasm3_ast.Statement]: @@ -186,6 +197,4 @@ def accept(self, visitor: QasmVisitor) -> None: unrolled_stmt_list = visitor.visit_basic_block(self._statements) final_stmt_list = visitor.finalize(unrolled_stmt_list) - self.unrolled_ast.statements = self._drop_global_phase( # type: ignore[assignment] - final_stmt_list - ) + self.unrolled_ast.statements = self.finalize(final_stmt_list) # type: ignore[assignment] diff --git a/src/pyqasm/modules/qasm3.py b/src/pyqasm/modules/qasm3.py index 41e3458b..23b9d1fd 100644 --- a/src/pyqasm/modules/qasm3.py +++ b/src/pyqasm/modules/qasm3.py @@ -89,4 +89,4 @@ def accept(self, visitor: QasmVisitor) -> None: unrolled_stmt_list = visitor.visit_basic_block(self._statements) final_stmt_list = visitor.finalize(unrolled_stmt_list) - self._unrolled_ast.statements = final_stmt_list # type: ignore[assignment] + self._unrolled_ast.statements = self.finalize(final_stmt_list) # type: ignore[assignment]