-
Notifications
You must be signed in to change notification settings - Fork 27
feat: support #pragma statements and preserve braket verbatim boxes #341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
89e2893
993cd8b
b3bf100
456e03b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -134,6 +134,8 @@ def mpl_draw( # pylint: disable=too-many-locals | |
| for s in program._statements: | ||
| if isinstance(s, ast.QuantumPhase): | ||
| global_phase += Qasm3ExprEvaluator.evaluate_expression(s.argument)[0] | ||
| elif isinstance(s, ast.Pragma): | ||
| continue # pragmas carry no timing information | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [L1] This branch is load-bearing but untested — Implementation · Low Rationale: without this Change requested: add one
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved in 456e03b — Verified it is load-bearing: deleting the One thing worth knowing about the motivation: a Braket verbatim program cannot be drawn today regardless of this branch — |
||
| else: | ||
| statements.append(s) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -143,6 +143,11 @@ def __init__( # pylint: disable=too-many-arguments | |||||||||||||||||
| # timeline within the box. Delays on disjoint qubits run in parallel, | ||||||||||||||||||
| # so box durations are validated against the per-qubit maximum. | ||||||||||||||||||
| self._box_delay_frames: list[dict[tuple[str, int], float]] = [] | ||||||||||||||||||
| # A 'braket verbatim' pragma applies to the box that immediately follows it. | ||||||||||||||||||
| # Gates inside such a box are emitted as written so that the device receives | ||||||||||||||||||
| # the exact native instructions the user asked for. | ||||||||||||||||||
| self._verbatim_pragma_pending: bool = False | ||||||||||||||||||
| self._in_verbatim_box: bool = False | ||||||||||||||||||
| self._in_extern_function: bool = False | ||||||||||||||||||
| self._openpulse_qubit_map: dict[str, set[str]] = {} | ||||||||||||||||||
| self._total_pulse_qubits: int = 0 | ||||||||||||||||||
|
|
@@ -188,6 +193,7 @@ def _construct_visit_map(self): | |||||||||||||||||
| qasm3_ast.ContinueStatement: self._visit_continue, | ||||||||||||||||||
| qasm3_ast.DelayInstruction: self._visit_delay_statement, | ||||||||||||||||||
| qasm3_ast.Box: self._visit_box_statement, | ||||||||||||||||||
| qasm3_ast.Pragma: self._visit_pragma, | ||||||||||||||||||
| qasm3_ast.CalibrationDefinition: self._visit_calibration_definition, | ||||||||||||||||||
| qasm3_ast.CalibrationStatement: self._visit_calibration_statement, | ||||||||||||||||||
| qasm3_ast.CalibrationGrammarDeclaration: self._visit_calibration_grammar_declaration, | ||||||||||||||||||
|
|
@@ -1257,8 +1263,9 @@ def _visit_custom_gate_operation( | |||||||||||||||||
| self._scope_manager.push_context(Context.GATE) | ||||||||||||||||||
|
|
||||||||||||||||||
| # Pause recording the depth of new gates because we are processing the | ||||||||||||||||||
| # definition of a custom gate here - handle the depth separately afterwards | ||||||||||||||||||
| self._recording_ext_gate_depth = gate_name in self._external_gates | ||||||||||||||||||
| # definition of a custom gate here - handle the depth separately afterwards. | ||||||||||||||||||
| # A verbatim gate is emitted as written, so it counts once, like an external gate. | ||||||||||||||||||
| self._recording_ext_gate_depth = self._in_verbatim_box or gate_name in self._external_gates | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [M1] Verbatim gate with a decomposition rule reports decomposed depth — Implementation · Medium (pre-existing; follow-up, not a blocker) Rationale: Root cause: This is pre-existing — on Change requested: file a follow-up against
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Follow-up filed as #352 and linked from the description. No code change here, per your call that it is not worth growing the diff. Reproduced both halves first: The description now carries a "Known gap" section instead of leaving it unmentioned. |
||||||||||||||||||
|
|
||||||||||||||||||
| result = [] | ||||||||||||||||||
| for gate_op in gate_definition_ops: | ||||||||||||||||||
|
|
@@ -1577,7 +1584,7 @@ def _visit_generic_gate_operation( # pylint: disable=too-many-branches, too-man | |||||||||||||||||
| for _ in range(power_value): | ||||||||||||||||||
| if isinstance(operation, qasm3_ast.QuantumPhase): | ||||||||||||||||||
| result.extend(self._visit_phase_operation(operation, inverse_value, ctrls)) | ||||||||||||||||||
| elif operation.name.name in self._external_gates: | ||||||||||||||||||
| elif self._in_verbatim_box or operation.name.name in self._external_gates: | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [M2] Custom gate in a verbatim box emits output that does not round-trip — Design / Maintenance · Medium (pre-existing in kind) Rationale: this branch makes every gate take the external path when inside a verbatim box, user-defined ones included. Given Pre-existing in kind — the same non-round-tripping output reproduces on Change requested: either reject a non-native, non-basis custom gate inside a verbatim box with a clear error, or document that verbatim bodies must contain only device-native gates. At minimum a line in the docs added by this PR.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved in 456e03b — took the documentation option. Confirmed the round-trip failure first:
|
||||||||||||||||||
| result.extend(self._visit_external_gate_operation(operation, inverse_value, ctrls)) | ||||||||||||||||||
|
Comment on lines
+1587
to
1588
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Count a custom verbatim gate as one emitted gate. When a custom gate is inside a verbatim box, Set the depth-recording state for verbatim custom gates too. Preserve and restore the previous state for nested custom gates. 🤖 Prompt for AI Agents |
||||||||||||||||||
| elif operation.name.name in self._custom_gates: | ||||||||||||||||||
| result.extend(self._visit_custom_gate_operation(operation, inverse_value, ctrls)) | ||||||||||||||||||
|
|
@@ -2934,6 +2941,42 @@ def _visit_delay_statement( | |||||||||||||||||
|
|
||||||||||||||||||
| return [statement] | ||||||||||||||||||
|
|
||||||||||||||||||
| @staticmethod | ||||||||||||||||||
| def _is_verbatim_pragma(statement: qasm3_ast.Pragma) -> bool: | ||||||||||||||||||
| """Check whether a pragma marks the following box as verbatim. | ||||||||||||||||||
|
|
||||||||||||||||||
| Args: | ||||||||||||||||||
| statement (qasm3_ast.Pragma): The pragma to inspect. | ||||||||||||||||||
|
|
||||||||||||||||||
| Returns: | ||||||||||||||||||
| bool: True for a 'braket verbatim' pragma, False for any other. | ||||||||||||||||||
| """ | ||||||||||||||||||
| return statement.command.split() == ["braket", "verbatim"] | ||||||||||||||||||
|
Comment on lines
+2944
to
+2954
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Add parameter and return documentation.
🤖 Prompt for AI AgentsSource: Coding guidelines |
||||||||||||||||||
|
|
||||||||||||||||||
| def _visit_pragma(self, statement: qasm3_ast.Pragma) -> list[qasm3_ast.Pragma]: | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [M3] Qubit-referencing pragmas silently desync from renumbering passes — Implementation · Medium Rationale: pass-through means no transform can rewrite pragma text, so a pragma that names qubits by index goes stale as soon as a renumbering pass runs. Two verified cases: After This is inherent to the pass-through design, and parsing vendor pragma grammar is not the suggestion. It is newly reachable, though — before this PR such programs were rejected outright — and it fails silently, producing a program that is valid QASM and wrong. Note this is distinct from the Change requested: document in the docs added here that pragma bodies are opaque and are not rewritten by qubit-renumbering passes (
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved in 456e03b — documented in the same Pragmas section. Verified both directions on this branch: The README now records that |
||||||||||||||||||
| """ | ||||||||||||||||||
| Visit a Pragma statement. | ||||||||||||||||||
|
|
||||||||||||||||||
| Pragmas carry vendor specific directives which pyqasm does not interpret, so they | ||||||||||||||||||
| are passed through unchanged. A 'braket verbatim' pragma additionally marks the box | ||||||||||||||||||
| that follows it, whose gates are then left as written instead of being decomposed. | ||||||||||||||||||
|
Comment on lines
+2960
to
+2962
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [L2] Docstring the honour-vs-forward asymmetry — Maintenance · Low Rationale: Change requested:
Suggested change
Separately, in this file: the module-level
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved in 456e03b — suggestion applied verbatim to the Left the |
||||||||||||||||||
| A verbatim pragma not followed by a box is honoured by nothing in pyqasm but is | ||||||||||||||||||
| still copied to the output, where the consumer applies it to whatever comes next. | ||||||||||||||||||
|
|
||||||||||||||||||
| Args: | ||||||||||||||||||
| statement (qasm3_ast.Pragma): The Pragma node to visit. | ||||||||||||||||||
| Returns: | ||||||||||||||||||
| list[qasm3_ast.Pragma]: The pragma, unmodified. | ||||||||||||||||||
| """ | ||||||||||||||||||
| logger.debug("Visiting pragma '%s'", statement.command) | ||||||||||||||||||
| if self._is_verbatim_pragma(statement): | ||||||||||||||||||
| self._verbatim_pragma_pending = True | ||||||||||||||||||
|
|
||||||||||||||||||
| if self._check_only: | ||||||||||||||||||
| return [] | ||||||||||||||||||
|
|
||||||||||||||||||
| return [statement] | ||||||||||||||||||
|
|
||||||||||||||||||
| def _visit_box_statement(self, statement: qasm3_ast.Box) -> list[qasm3_ast.Statement]: | ||||||||||||||||||
| """ | ||||||||||||||||||
| Visit a Box statement. | ||||||||||||||||||
|
|
@@ -2943,6 +2986,9 @@ def _visit_box_statement(self, statement: qasm3_ast.Box) -> list[qasm3_ast.State | |||||||||||||||||
| list[qasm3_ast.Statement]: The list of statements generated by the Box statement. | ||||||||||||||||||
| """ | ||||||||||||||||||
| statements = [] | ||||||||||||||||||
| outer_verbatim = self._in_verbatim_box | ||||||||||||||||||
| self._in_verbatim_box = outer_verbatim or self._verbatim_pragma_pending | ||||||||||||||||||
| self._verbatim_pragma_pending = False | ||||||||||||||||||
| _box_time_var = statement.duration | ||||||||||||||||||
| box_duration_val = 0 | ||||||||||||||||||
| if _box_time_var is not None: | ||||||||||||||||||
|
|
@@ -2976,6 +3022,9 @@ def _visit_box_statement(self, statement: qasm3_ast.Box) -> list[qasm3_ast.State | |||||||||||||||||
| self._scope_manager.restore_context() | ||||||||||||||||||
| self._scope_manager.decrement_scope_level() | ||||||||||||||||||
| self._scope_manager.pop_scope() | ||||||||||||||||||
| self._in_verbatim_box = outer_verbatim | ||||||||||||||||||
|
argus-eye[bot] marked this conversation as resolved.
|
||||||||||||||||||
| # a marker left behind by the body must not reach the next box | ||||||||||||||||||
| self._verbatim_pragma_pending = False | ||||||||||||||||||
|
|
||||||||||||||||||
| delay_frame = self._box_delay_frames.pop() | ||||||||||||||||||
| if _box_time_var and box_duration_val and delay_frame: | ||||||||||||||||||
|
|
@@ -3265,18 +3314,26 @@ def _visit_include(self, include: qasm3_ast.Include) -> list[qasm3_ast.Statement | |||||||||||||||||
|
|
||||||||||||||||||
| return [include] | ||||||||||||||||||
|
|
||||||||||||||||||
| def visit_statement(self, statement: qasm3_ast.Statement) -> list[qasm3_ast.Statement]: | ||||||||||||||||||
| def visit_statement( | ||||||||||||||||||
| self, statement: qasm3_ast.Statement | qasm3_ast.Pragma | ||||||||||||||||||
| ) -> list[qasm3_ast.Statement]: | ||||||||||||||||||
| """Visit a statement element. | ||||||||||||||||||
|
|
||||||||||||||||||
| Args: | ||||||||||||||||||
| statement (qasm3_ast.Statement): The statement to visit. | ||||||||||||||||||
| statement (qasm3_ast.Statement | qasm3_ast.Pragma): The statement to visit. | ||||||||||||||||||
|
|
||||||||||||||||||
| Returns: | ||||||||||||||||||
| None | ||||||||||||||||||
| """ | ||||||||||||||||||
| logger.debug("Visiting statement '%s'", str(statement)) | ||||||||||||||||||
| result = [] | ||||||||||||||||||
|
|
||||||||||||||||||
| if not isinstance(statement, qasm3_ast.Box): | ||||||||||||||||||
| # a pending verbatim pragma only carries over to a box directly following it. | ||||||||||||||||||
| # Clearing before dispatch lets a verbatim pragma re-arm the flag for itself, | ||||||||||||||||||
| # while any other statement - another pragma included - drops it. | ||||||||||||||||||
| self._verbatim_pragma_pending = False | ||||||||||||||||||
|
|
||||||||||||||||||
| visitor_function = self._visit_map.get(type(statement)) | ||||||||||||||||||
| if visitor_function: | ||||||||||||||||||
| if isinstance(statement, qasm3_ast.ExpressionStatement): | ||||||||||||||||||
|
|
@@ -3293,11 +3350,13 @@ def visit_statement(self, statement: qasm3_ast.Statement) -> list[qasm3_ast.Stat | |||||||||||||||||
| ) | ||||||||||||||||||
| return result | ||||||||||||||||||
|
|
||||||||||||||||||
| def visit_basic_block(self, stmt_list: list[qasm3_ast.Statement]) -> list[qasm3_ast.Statement]: | ||||||||||||||||||
| def visit_basic_block( | ||||||||||||||||||
| self, stmt_list: Sequence[qasm3_ast.Statement | qasm3_ast.Pragma] | ||||||||||||||||||
| ) -> list[qasm3_ast.Statement]: | ||||||||||||||||||
| """Visit a basic block of statements. | ||||||||||||||||||
|
|
||||||||||||||||||
| Args: | ||||||||||||||||||
| stmt_list (list[qasm3_ast.Statement]): The list of statements to visit. | ||||||||||||||||||
| stmt_list (Sequence[qasm3_ast.Statement | qasm3_ast.Pragma]): The statements to visit. | ||||||||||||||||||
|
|
||||||||||||||||||
| Returns: | ||||||||||||||||||
| list[qasm3_ast.Statement]: The list of unrolled statements. | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Complete the new API documentation and annotations.
Qasm3Printer.visit_Pragma()has no docstring.dumps()does not documentnode,kwargs, or its return value in the required format. Annotatekwargsas well.🤖 Prompt for AI Agents
Source: Coding guidelines