Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +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 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))
Expand Down
24 changes: 19 additions & 5 deletions src/pyqasm/modules/qasm2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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

Expand All @@ -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)
10 changes: 4 additions & 6 deletions tests/qasm2/test_conditional_body.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)"""
Comment on lines +67 to +69

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add the test annotation and return documentation.

Add -> None. Add a Returns section that documents the None return value.

As per coding guidelines, **/*.py requires type annotations for all functions and docstrings that explain return values.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/qasm2/test_conditional_body.py` around lines 67 - 69, Add the return
type annotation -> None to test_conditional_rzz_survives_refiltering, and extend
its docstring with a Returns section documenting that the test returns None.

Source: Coding guidelines

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(
Expand Down
107 changes: 107 additions & 0 deletions tests/qasm2/test_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)"""
Comment on lines +70 to +72

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add test function annotations and return documentation.

Add -> None to each test function. Add a Returns section that documents the None return value.

As per coding guidelines, **/*.py requires type annotations for all functions and docstrings that explain return values.

Also applies to: 98-100, 126-127, 146-148, 163-165

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/qasm2/test_operations.py` around lines 70 - 72, Update all affected
test functions in this file, including test_rzz_unrolls_without_gphase and the
additional referenced tests, to declare -> None and extend each docstring with a
Returns section documenting that the function returns None.

Source: Coding guidelines

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
Expand Down
Loading