From c2963689b98b71679284d27471c9df67ae90a782 Mon Sep 17 00:00:00 2001 From: Ryan Hill Date: Fri, 24 Jul 2026 12:36:07 -0400 Subject: [PATCH 1/2] fix: emit fresh operand nodes from gate decompositions Gate decomposition functions passed the same IndexedIdentifier objects into every statement they emitted, so in-place index rewrites in remove_idle_qubits() and reverse_qubit_order() mutated a shared node once per referencing statement, raising KeyError whenever the remap was not the identity. Statement constructors in maps/gates.py and Decomposer now deep-copy their qubit operands so every emitted statement owns its nodes. Fixes #333 --- CHANGELOG.md | 1 + src/pyqasm/decomposer.py | 6 +- src/pyqasm/maps/gates.py | 24 ++++++-- tests/qasm3/test_transformations.py | 89 +++++++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63fac09e..01615b0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Types of changes: ### Removed ### Fixed +- 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 deep-copy their qubit operands so every emitted statement owns its nodes. ([#333](https://github.com/qBraid/pyqasm/issues/333)) - Fixed `box` duration validation summing `delay` durations across all qubits instead of tracking each qubit's timeline. Delays on disjoint qubits run in parallel, so `box[300ns] { delay[200ns] q[0]; delay[200ns] q[1]; }` was rejected while the identical schedule written as a broadcast delay (`delay[200ns] q;`) was accepted. Delays are now accumulated per qubit and the box is validated against the busiest single timeline; the error message names the offending qubit. Nested boxes now also contribute their declared duration to the enclosing box's timelines (previously the accumulator was reset when an inner box closed, dropping all inner delay accounting). ([#330](https://github.com/qBraid/pyqasm/pull/330)) - Fixed `reset` on a physical qubit rewriting the operand to the internal pulse register, e.g. `reset $2;` unrolled to `reset __PYQASM_QUBITS__[2];`. That names a register the program never declares, so the unrolled output did not round-trip through `dumps()`/`loads()`, and the qubit was never registered (a program whose only operation was `reset $3;` reported `num_qubits == 0`). Physical qubits are now kept as-is in plain QASM programs, matching how gate and measurement operands already treat them; the rename is still applied for OpenPulse programs, where the pulse visitor expects it. ([#325](https://github.com/qBraid/pyqasm/pull/325)) - Fixed `measure` and `reset` on a user register whose name merely starts with the reserved internal register name (e.g. `__PYQASM_QUBITS__foo`) being mistaken for the internal register itself. Such statements were short-circuited out of unrolling and emitted verbatim, so `c = measure __PYQASM_QUBITS__foo;` was never expanded per qubit and `reset __PYQASM_QUBITS__foo;` silently reset nothing. The register is now matched on its exact name, or on the name followed by an index or slice. ([#325](https://github.com/qBraid/pyqasm/pull/325)) diff --git a/src/pyqasm/decomposer.py b/src/pyqasm/decomposer.py index d302f938..9c41bf89 100644 --- a/src/pyqasm/decomposer.py +++ b/src/pyqasm/decomposer.py @@ -16,6 +16,8 @@ Definition of the Decomposer class """ +from copy import deepcopy + import openqasm3.ast as qasm3_ast from openqasm3.ast import BranchingStatement, QuantumGate @@ -133,7 +135,9 @@ def _get_decomposed_gates(cls, decomposition_rules, statement, gate): modifiers=[], name=qasm3_ast.Identifier(name=rule["gate"]), arguments=arguments, - qubits=qubits, + # copy so the emitted gates do not share operand nodes with each + # other or with the source statement (see #333) + qubits=[deepcopy(qubit) for qubit in qubits], ) decomposed_gates.append(new_gate) diff --git a/src/pyqasm/maps/gates.py b/src/pyqasm/maps/gates.py index 0d074e43..7d02f1de 100644 --- a/src/pyqasm/maps/gates.py +++ b/src/pyqasm/maps/gates.py @@ -19,6 +19,7 @@ """ +from copy import deepcopy from typing import Callable import numpy as np @@ -30,6 +31,17 @@ from pyqasm.maps.expressions import CONSTANTS_MAP +def _fresh_qubits(*qubits: IndexedIdentifier) -> list[IndexedIdentifier]: + """Deep-copy qubit operands so every emitted statement owns its nodes. + + Decomposition functions pass the same operand nodes to each statement they + emit; without copies, transforms that later rewrite qubit indices in place + (e.g. ``remove_idle_qubits``, ``reverse_qubit_order``) would mutate a + shared node once per referencing statement. + """ + return [deepcopy(qubit) for qubit in qubits] + + def u3_gate( theta: int | float, phi: int | float, @@ -113,7 +125,9 @@ def global_phase_gate(theta: float, qubit_list: list[IndexedIdentifier]) -> list """ return [ QuantumPhase( - argument=FloatLiteral(value=theta), qubits=qubit_list, modifiers=[] # type: ignore + argument=FloatLiteral(value=theta), + qubits=_fresh_qubits(*qubit_list), # type: ignore + modifiers=[], ) ] @@ -702,7 +716,7 @@ def ccx_gate_op( modifiers=[], name=Identifier(name="ccx"), arguments=[], - qubits=[qubit0, qubit1, qubit2], + qubits=_fresh_qubits(qubit0, qubit1, qubit2), ) ] @@ -905,7 +919,7 @@ def one_qubit_gate_op(gate_name: str, qubit_id: IndexedIdentifier) -> list[Quant modifiers=[], name=Identifier(name=gate_name), arguments=[], - qubits=[qubit_id], + qubits=_fresh_qubits(qubit_id), ) ] @@ -918,7 +932,7 @@ def one_qubit_rotation_op( modifiers=[], name=Identifier(name=gate_name), arguments=[FloatLiteral(value=rotation)], - qubits=[qubit_id], + qubits=_fresh_qubits(qubit_id), ) ] @@ -931,7 +945,7 @@ def two_qubit_gate_op( modifiers=[], name=Identifier(name=gate_name.lower()), arguments=[], - qubits=[qubit_id1, qubit_id2], + qubits=_fresh_qubits(qubit_id1, qubit_id2), ) ] diff --git a/tests/qasm3/test_transformations.py b/tests/qasm3/test_transformations.py index a8c0eabe..18205a3c 100644 --- a/tests/qasm3/test_transformations.py +++ b/tests/qasm3/test_transformations.py @@ -17,7 +17,12 @@ """ +import pytest + +from pyqasm.analyzer import Qasm3Analyzer +from pyqasm.elements import BasisSet from pyqasm.entrypoint import dumps, loads +from pyqasm.maps import QUANTUM_STATEMENTS from tests.utils import check_unrolled_qasm @@ -135,6 +140,90 @@ def test_reverse_qubit_order_qasm3(): check_unrolled_qasm(dumps(module), expected_qasm3_str) +def test_reverse_qubit_order_gate_decomposition(): + """Test reverse_qubit_order on a decomposed gate whose statements previously + shared operand nodes (issue #333)""" + qasm3_str = """ + OPENQASM 3.0; + include "stdgates.inc"; + qubit[3] q; + crz(0.5) q[1], q[2]; + """ + + expected_qasm3_str = """ + OPENQASM 3.0; + include "stdgates.inc"; + qubit[3] q; + rz(0.25) q[0]; + rx(1.5707963267948966) q[0]; + rz(3.141592653589793) q[0]; + rx(1.5707963267948966) q[0]; + rz(3.141592653589793) q[0]; + cx q[1], q[0]; + rz(-0.25) q[0]; + rx(1.5707963267948966) q[0]; + rz(3.141592653589793) q[0]; + rx(1.5707963267948966) q[0]; + rz(3.141592653589793) q[0]; + cx q[1], q[0]; + """ + + module = loads(qasm3_str) + module.unroll() + module.reverse_qubit_order() + check_unrolled_qasm(dumps(module), expected_qasm3_str) + + +def _assert_no_shared_operand_nodes(module): + """Assert no two quantum statements in the unrolled AST share an operand node""" + seen_bits: set[int] = set() + seen_indices: set[int] = set() + for statement in module._unrolled_ast.statements: + if isinstance(statement, QUANTUM_STATEMENTS): + for bit in Qasm3Analyzer.get_op_bit_list(statement): + assert id(bit) not in seen_bits, f"operand node shared: {bit}" + seen_bits.add(id(bit)) + index_node = bit.indices[0][0] + assert id(index_node) not in seen_indices, f"index node shared: {bit}" + seen_indices.add(id(index_node)) + + +@pytest.mark.parametrize( + "operation", + [ + "crz(0.5) q[1], q[2];", + "crx(0.5) q[0], q[1];", + "c4x q[0], q[1], q[2], q[3], q[4];", + "ecr q[0], q[1];", + "inv @ crz(0.5) q[1], q[2];", + ], +) +def test_unroll_emits_fresh_operand_nodes(operation): + """Test that unroll() never emits statements sharing operand nodes, so in-place + transformations remap each operand exactly once (issues #331, #333)""" + qasm3_str = f""" + OPENQASM 3.0; + include "stdgates.inc"; + qubit[5] q; + {operation} + """ + module = loads(qasm3_str) + module.unroll() + _assert_no_shared_operand_nodes(module) + + +def test_rebase_emits_fresh_operand_nodes(): + """Test that rebase() never emits statements sharing operand nodes (issue #333)""" + qasm3_str = """ + OPENQASM 3.0; + include "stdgates.inc"; + qubit[3] q; + crz(0.5) q[1], q[2]; + """ + module = loads(qasm3_str).rebase(BasisSet.ROTATIONAL_CX) + _assert_no_shared_operand_nodes(module) + + def test_populate_idle_qubits_qasm3(): """Test the populate idle qubits function for qasm3 string""" From e4293d6f655cff941e9bbc5f826482d11bc73aa6 Mon Sep 17 00:00:00 2001 From: Ryan Hill Date: Fri, 24 Jul 2026 12:39:15 -0400 Subject: [PATCH 2/2] fix mypy: widen _fresh_qubits return type for QuantumGate.qubits invariance --- src/pyqasm/maps/gates.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pyqasm/maps/gates.py b/src/pyqasm/maps/gates.py index 7d02f1de..63fd6602 100644 --- a/src/pyqasm/maps/gates.py +++ b/src/pyqasm/maps/gates.py @@ -31,7 +31,7 @@ from pyqasm.maps.expressions import CONSTANTS_MAP -def _fresh_qubits(*qubits: IndexedIdentifier) -> list[IndexedIdentifier]: +def _fresh_qubits(*qubits: IndexedIdentifier) -> list[IndexedIdentifier | Identifier]: """Deep-copy qubit operands so every emitted statement owns its nodes. Decomposition functions pass the same operand nodes to each statement they