diff --git a/CHANGELOG.md b/CHANGELOG.md index 63fac09..01615b0 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 d302f93..9c41bf8 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 0d074e4..63fd660 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 | Identifier]: + """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 a8c0eab..18205a3 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"""