Fixes #183 by arunjmoorthy · Pull Request #227 · qBraid/pyqasm

arunjmoorthy · 2025-07-06T20:37:14Z

Summary of changes

This is a draft PR. This does not solve all of the issues in issue 183. Look at the bottom of the visitor.py file. There, some of the functions are implemented to parse the defcal block, which has several restrictions. Additionally, one openpulse test example file has been added to help with iterating and testing.

github-actions · 2025-07-06T20:37:24Z

👋 Hey there! It looks like the changelog might need an update.

Please take a moment to edit the CHANGELOG.md with:

A brief, one-to-two sentence summary of your changes.
A link back to this PR for reference.
(Optional) A small working example if you've added new features.

TheGupta2012 · 2025-07-08T10:38:28Z

src/pyqasm/entrypoint.py

+                program = openqasm3.parse(program)
+        except (openqasm3.parser.QASM3ParsingError, openpulse.parser.OpenPulseParsingError) as err:
+            raise ValidationError(f"Failed to parse the OpenQASM/Openpulse string: {err}") from err
+    elif not isinstance(program, openqasm3.ast.Program): 


Shouldn't we check for both the ast types here i.e. for openqasm3.ast.Program and openpulse.ast.Program?

I think we don’t need to parse inputs separately; we can simply call openpulse.parser.parse_openpulse() inside calibration functions to handle their statement body.
Suggestion: since openpulse.parse() parses both generic QASM AST's and OpenPulse ASTs, we can replace our current call to openqasm.parse() with openpulse.parse().

TheGupta2012 · 2025-07-08T10:38:36Z

src/pyqasm/modules/qasm3.py

            visitor (QasmVisitor): The visitor to accept
        """
        unrolled_stmt_list = visitor.visit_basic_block(self._statements)
+        # print("--------------------------------")


remove comments

TheGupta2012 · 2025-07-08T10:38:57Z

src/pyqasm/visitor.py

+
+
+
+    # You'll also need these other visitor methods for your visit map:


TheGupta2012 · 2025-07-08T10:39:09Z

src/pyqasm/visitor.py

+        print("Calibration Statement Structure:")
+        print("Statement: ", statement)


TheGupta2012 · 2025-07-08T10:39:17Z

src/pyqasm/visitor.py

+        print("Calibration Grammar Declaration Structure:")
+        print("Declaration: ", declaration)


TheGupta2012 · 2025-07-08T10:39:42Z

src/pyqasm/visitor.py

+        print("--------------------------------")
+        print("Gate name: ", gate_name)
+        print("--------------------------------")


remove, or move to logging level DEBUG

TheGupta2012 · 2025-07-08T10:40:03Z

src/pyqasm/visitor.py

+        print("--------------------------------")
+        print("Parsing physical qubits: ", qubits)


remove or replace with logger

TheGupta2012 · 2025-07-08T10:40:13Z

src/pyqasm/visitor.py

+            print("--------------------------------")
+            print("Qubit: ", qubit)
+            print("--------------------------------")


TheGupta2012 · 2025-07-08T10:40:22Z

src/pyqasm/visitor.py

+                )
+
+        print("--------------------------------")
+        return physical_qubits


TheGupta2012 · 2025-07-08T10:40:29Z

src/pyqasm/visitor.py

+        print("--------------------------------")
+        print("Validating calibration arguments: ", arguments)
+        print("--------------------------------")


remove or replace with logger

TheGupta2012 · 2025-07-08T10:40:42Z

src/pyqasm/visitor.py

+            print("--------------------------------")
+            print("Argument: ", arg)
+            print("--------------------------------")


TheGupta2012 · 2025-07-08T10:40:51Z

src/pyqasm/visitor.py

+        print("--------------------------------")
+        print("Validating calibration body: ", body)
+        print("--------------------------------")


remove or replace with logger

TheGupta2012 · 2025-07-08T10:40:57Z

src/pyqasm/visitor.py

+            print("--------------------------------")
+            print("Validating calibration body: ", body)
+            print("--------------------------------")


remove or replace with logger

TheGupta2012 · 2025-07-08T10:43:35Z

src/pyqasm/visitor.py

+        grammar_name = declaration.name
+
+        # Validate supported calibration grammars
+        supported_grammars = {"openpulse"}  # Add more as needed


Better to make this a constant and move to maps sub-package

TheGupta2012 · 2025-07-08T10:49:11Z

src/pyqasm/visitor.py

+                )
+
+            try:
+                physical_qubit_id = int(qubit_name[1:])


Please check that the qubit index is >=0 here AND , if device_qubits are specified (see PR #222 for reference), within the range of the total number of the qubits present on the device for which this program is intended

Moreover, as stated above, not all args will be having a prefixed $ sign. Will need to treat those as an identifier then****

TheGupta2012 · 2025-07-08T10:50:12Z

src/pyqasm/visitor.py

+        print("--------------------------------")
+        return physical_qubits
+
+    def _validate_calibration_arguments(self, arguments: list, gate_name: str, definition: pulse_ast.CalibrationDefinition) -> None:


Should be refactored to the Qasm3Validator module

TheGupta2012 · 2025-07-08T10:55:49Z

src/pyqasm/visitor.py

+                )
+            seen_qubits.add(qubit_name)
+
+            if not qubit_name.startswith('$'):


We can't assume that the qubit arguments will only be composed of physical qubits. If you see the example -

defcal rz(angle[20] theta) q { ... } // we've defined ``rz`` on arbitrary physical qubits, so we can do: rz(3.14) $0; rz(3.14) $1;

the defcals blocks can also be defined for identifiers , meaning that for all physical qubits, we are to follow the given defcal body.

TheGupta2012 · 2025-07-08T10:58:08Z

src/pyqasm/visitor.py

+            'DurationType': 64, 'ComplexType': 128
+        }.get(type_name, 32)
+
+    def _validate_openpulse_declaration_type(self, stmt, gate_name: str, definition) -> None:


refactor the the Qasm3Validator

TheGupta2012 · 2025-07-08T10:58:48Z

src/pyqasm/visitor.py

+                span=definition.span,
+            )
+
+    def _basic_pulse_declaration_validation(self, stmt, gate_name: str, definition) -> None:


Again, refactor to the Qasm3Validator module

TheGupta2012 · 2025-07-08T10:58:59Z

src/pyqasm/visitor.py

+        )
+        self._add_var_in_scope(variable)
+
+    def _validate_openpulse_function_call(self, stmt, gate_name: str, definition) -> None:


Refactor to the Qasm3Validator module

TheGupta2012 · 2025-07-08T10:59:31Z

src/pyqasm/visitor.py

+        if hasattr(stmt, 'expression') and hasattr(stmt.expression, 'name'):
+            func_name = stmt.expression.name.name
+
+            openpulse_functions = {


define in the maps/pulse.py module

TheGupta2012 · 2025-07-08T10:59:46Z

src/pyqasm/visitor.py

+    def _validate_openpulse_declaration_type(self, stmt, gate_name: str, definition) -> None:
+        """Validate that declaration uses valid openpulse types."""
+        type_name = type(stmt.type).__name__
+        valid_types = {'WaveformType', 'FrameType', 'PortType', 'IntType', 'FloatType', 'AngleType', 'DurationType'}


define in the maps/pulse.py module

TheGupta2012 · 2025-07-08T11:02:09Z

src/pyqasm/visitor.py

+            print("Validating calibration body: ", body)
+            print("--------------------------------")
+            for stmt in body:
+                self._validate_calibration_statement_dispatch(stmt, gate_name, definition)


Let's handle exceptions here. We can extract the type name and catch the raised exceptions instead of duplicating the raising in the dispatch method

TheGupta2012 · 2025-07-08T11:03:40Z

src/pyqasm/visitor.py

+    def _get_calibration_arg_size(self, arg_type) -> int:
+        """Get size for calibration argument type."""
+        type_name = type(arg_type).__name__
+        return {


Move the dict to maps/pulse.py and directly use the .get method on it. Will ensure that we're not redefining it again and again during fn call.

TheGupta2012 · 2025-07-08T11:04:44Z

README.md

 pip install pyqasm
 ```

+### Optional Dependencies


Please remove these README changes, I have already consolidated these extra installs under 1 head.

TheGupta2012

Hi @arunjmoorthy , thank you for the great work! I have given some comments regarding the refactor of code and about adding more validations.

Also, please add unit tests for the openpulse grammar. I know that there are a LOT of cases, but we'll need to cover all of them in order to achieve our goal of full QASM coverage!

@vinayswamik do have a look at the PR, maybe you guys can collaborate on this?

arunjmoorthy added 8 commits May 26, 2025 13:35

Fix qBraid#182: added pulse dependency/docs

bf65034

readme/test_import updated

9449856

test_import updated

b56be84

fixed import order

551bd1d

fixed black formatting

f8ef8fb

added license header

800251c

structure for defcal

008f8eb

defcal block parsing structure

bce09b4

arunjmoorthy requested a review from TheGupta2012 as a code owner July 6, 2025 20:37

TheGupta2012 reviewed Jul 8, 2025

View reviewed changes

src/pyqasm/visitor.py

# You'll also need these other visitor methods for your visit map:

Copy link
Copy Markdown

Member

TheGupta2012 Jul 8, 2025

Choose a reason for hiding this comment

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

remove

TheGupta2012 reviewed Jul 8, 2025

View reviewed changes

src/pyqasm/visitor.py

)

print("--------------------------------")

return physical_qubits

Copy link
Copy Markdown

Member

TheGupta2012 Jul 8, 2025

Choose a reason for hiding this comment

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

remove

TheGupta2012 reviewed Jul 8, 2025

View reviewed changes

TheGupta2012 requested changes Jul 8, 2025

View reviewed changes

arunjmoorthy closed this Jul 16, 2025

arunjmoorthy deleted the issue183 branch July 16, 2025 06:33




		# You'll also need these other visitor methods for your visit map:

		print("Calibration Statement Structure:")
		print("Statement: ", statement)

		print("Calibration Grammar Declaration Structure:")
		print("Declaration: ", declaration)

		print("--------------------------------")
		print("Parsing physical qubits: ", qubits)

Conversation

arunjmoorthy commented Jul 6, 2025

Summary of changes

Uh oh!

github-actions bot commented Jul 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheGupta2012 Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheGupta2012 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

TheGupta2012 Jul 8, 2025 •

edited

Loading

TheGupta2012 left a comment •

edited

Loading