PyQASM Integration for sdk by TheGupta2012 · Pull Request #808 · qBraid/qBraid

TheGupta2012 · 2024-10-24T09:21:41Z

Summary of changes

Changes to the `qbraid.passes.qasm`

Removed analyze.py, format.py and unfold.py modules and migrated functionality and respective tests to pyqasm
Functions exclusively required by the sdk in compat.py and decompose.py are kept as is, others moved to pyqasm along with the tests
OpenQasm2Program and OpenQasm3Program classes now use the pyqasm methods
TODO : rebase method and corresponding tests are marked to be moved to the pyqasm module as part of the rebase PR

Changes to the `qbraid.programs`

qbraid/programs/typer.py catches pyqasm parsing exceptions and raises qbraid custom qasm exceptions. Relevant tests are updated.

Updates to coverage for transpiler

Only MS gate in braket is not supported right now, due to a bug in implementation.

github-actions · 2024-10-24T09:21:55Z

👋 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 · 2024-11-18T08:15:31Z

Will need to release pyqasm==0.1.0 for tests to pass. Passing locally with pyqasm installed from source in testing env

ryanhill1 · 2024-12-03T17:21:27Z

+    ms(1.1,0,0) q[0], q[1];
    """,
            "Invalid phase value",
        ),
        (
            """
    OPENQASM 3.0;
    qubit[2] q;
-    ms(0,-1.5) q[0], q[1];
+    ms(0,-1.5,0) q[0], q[1];


If OpenQASM supports optional argument to gate definitions, we will want to test both including optional argument and not including it. And if a default value is required, it should be 0.25, not 0.

Afaik, OpenQASM does not support optional args to gates.

Either we have two gate definitions for ms with distinct names or,

Stick to the max number of args and use x in the arg which is not required such that the term using this arg vanishes when we provide x.

Note that 0.25 might be a default for ionq but not for other providers which support ms gate.

Summarizing our conversation about this and putting on record: I think we decided best approach was to add support for both a two and a three parameter ms gate in pyqasm, distinguishing between the two based on the include statement e.g. with a custom ionqnative.inc to signal 0.25 default

touch ups bump pyqasm

codecov · 2024-12-05T10:52:06Z

Codecov Report

Attention: Patch coverage is 97.24771% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...nspiler/conversions/openqasm3/openqasm3_to_ionq.py	62.50%	3 Missing ⚠️

📢 Thoughts on this report? Let us know!

ryanhill1

rename_qasm_registers was removed with release 0.8.7 so you can take that function out. Additionally, need to satisfy the following code coverage which has dropped. There likely have been testing functions deleted that you may be able to reuse. Because all these had tests running previously

qbraid/passes/qasm/compat.py                                     117     13    89%   136, 159-160, 185, 195, 201-202, 213, 224-230
qbraid/passes/qasm/decompose.py                                   89      1    99%   245
qbraid/programs/gate_model/pytket.py                              84      1    99%   112
qbraid/programs/gate_model/qasm3.py                               55      4    93%   63, 87, 93, 98
qbraid/transpiler/conversions/cirq/cirq_quil_output.py           249      6    98%   366-367, 376-377, 386-387
qbraid/transpiler/conversions/openqasm3/openqasm3_to_ionq.py     107     10    91%   101-102, 148-149, 165-166, 193-194, 210, 248
qbraid/transpiler/conversions/qasm2/cirq_custom.py                73     13    82%   61-65, 123, 126-127, 147-151
qbraid/transpiler/conversions/qasm2/qasm2_to_qasm3.py              9      1    89%   32

ryanhill1

These updates look great! It's getting super close. As of your latest commit, it still looks like we're missing ~21 lines of code coverage:

Name                                                           Stmts   Miss  Cover   Missing
--------------------------------------------------------------------------------------------
qbraid/passes/qasm/compat.py                                     127      2    98%   222-223
qbraid/passes/qasm/decompose.py                                   91      1    99%   253
qbraid/programs/gate_model/pytket.py                              84      1    99%   112
qbraid/transpiler/conversions/cirq/cirq_quil_output.py           249      6    98%   366-367, 376-377, 386-387
qbraid/transpiler/conversions/openqasm3/openqasm3_to_ionq.py      98      4    96%   102-103, 195, 233
qbraid/transpiler/conversions/qasm2/cirq_custom.py                73      7    90%   61-65, 126-127
--------------------------------------------------------------------------------------------
TOTAL                                                           7004     21    99%

I know it's annoying, but we need to meet that pytest standard before merging. Like I mentioned before, easiest way is probably just to look through the tests that have been deleted, find the ones that previously hit the lines that are now missing, and re-work them to fit the new format. Or, if you think it would be quicker to write new tests from scratch for this missed lines, that works too.

ryanhill1 · 2024-12-11T16:11:57Z

+    ms(1.1,0,0) q[0], q[1];
    """,
            "Invalid phase value",
        ),
        (
            """
    OPENQASM 3.0;
    qubit[2] q;
-    ms(0,-1.5) q[0], q[1];
+    ms(0,-1.5,0) q[0], q[1];


Summarizing our conversation about this and putting on record: I think we decided best approach was to add support for both a two and a three parameter ms gate in pyqasm, distinguishing between the two based on the include statement e.g. with a custom ionqnative.inc to signal 0.25 default

ryanhill1 · 2024-12-11T16:24:43Z

Let's also bump project version to 0.9.0.dev in this PR.

And also, could you describe in more detail the braket MS gate bug?

TheGupta2012 · 2024-12-12T06:57:04Z

These updates look great! It's getting super close. As of your latest commit, it still looks like we're missing ~21 lines of code coverage:
Name                                                           Stmts   Miss  Cover   Missing
--------------------------------------------------------------------------------------------
qbraid/passes/qasm/compat.py                                     127      2    98%   222-223
qbraid/passes/qasm/decompose.py                                   91      1    99%   253
qbraid/programs/gate_model/pytket.py                              84      1    99%   112
qbraid/transpiler/conversions/cirq/cirq_quil_output.py           249      6    98%   366-367, 376-377, 386-387
qbraid/transpiler/conversions/openqasm3/openqasm3_to_ionq.py      98      4    96%   102-103, 195, 233
qbraid/transpiler/conversions/qasm2/cirq_custom.py                73      7    90%   61-65, 126-127
--------------------------------------------------------------------------------------------
TOTAL                                                           7004     21    99%
I know it's annoying, but we need to meet that pytest standard before merging. Like I mentioned before, easiest way is probably just to look through the tests that have been deleted, find the ones that previously hit the lines that are now missing, and re-work them to fit the new format. Or, if you think it would be quicker to write new tests from scratch for this missed lines, that works too.

Okay, I requested re-review as I saw that codecov bot showed all modified lines are covered. Will add more tests!

TheGupta2012 · 2024-12-13T13:03:55Z

Let's also bump project version to 0.9.0.dev in this PR.

And also, could you describe in more detail the braket MS gate bug?

The braket MS bug was coming while testing coverage from braket to qiskit. I checked the unitary matrices for Qiskit MS Gate (XX interaction is equivalent to 2 qubit MS gate for qiskit) and braket MS gate. The difference is the phase angle in the sine terms but a weird thing is that it passes for other packages.

TheGupta2012 · 2024-12-13T13:11:26Z

Also any idea why is the coverage for 3.10 and 3.11 different from 3.12 CI / Test action?

ryanhill1 · 2024-12-13T14:53:40Z

@TheGupta2012 Yeah it's because pyquil and pyqubo aren't compatible with Python 3.12. So when tox.ini does it's setup using requirements-dev.txt those dependencies are only installed if python < 3.12, and thus those pytests are skipped

TheGupta2012 requested a review from ryanhill1 as a code owner October 24, 2024 09:21

TheGupta2012 marked this pull request as draft October 24, 2024 09:22

TheGupta2012 force-pushed the pyqasm-integration branch from c325b14 to c7a4b40 Compare November 7, 2024 13:38

TheGupta2012 force-pushed the pyqasm-integration branch 2 times, most recently from 73d6dbb to b1d48a2 Compare November 18, 2024 08:14

TheGupta2012 marked this pull request as ready for review November 18, 2024 08:14

ryanhill1 requested changes Nov 19, 2024

View reviewed changes

TheGupta2012 mentioned this pull request Nov 20, 2024

Add qBraid SDK changes qBraid/pyqasm#86

Merged

ryanhill1 requested changes Dec 3, 2024

View reviewed changes

TheGupta2012 force-pushed the pyqasm-integration branch 6 times, most recently from a968763 to a8eb767 Compare December 5, 2024 07:19

TheGupta2012 requested a review from ryanhill1 December 5, 2024 07:30

TheGupta2012 force-pushed the pyqasm-integration branch from d69487c to 7d0b4c6 Compare December 5, 2024 08:31

TheGupta2012 added 11 commits December 5, 2024 15:41

start integration

c3f7b1a

remove_measurement

d0b8f4b

refactor extract version

5574932

mark tests and update more code

51eef9c

update more code and tests

205790b

finalise migration

422599e

touch ups

d1e03c6

review changes

8fee9e9

more changes + fix tests

f5267b1

review changes 2

c757bad

review changes 3

50125c6

touch ups bump pyqasm

fix tests

bbe7254

TheGupta2012 force-pushed the pyqasm-integration branch from 7d0b4c6 to bbe7254 Compare December 5, 2024 10:44

ryanhill1 requested changes Dec 5, 2024

View reviewed changes

TheGupta2012 added 3 commits December 10, 2024 23:15

add pyqasm v0.1.0

baa52ed

Merge branch 'main' into pyqasm-integration

de528a0

merge main and bump coverage

274447e

TheGupta2012 force-pushed the pyqasm-integration branch from 6fb7ccc to 274447e Compare December 11, 2024 09:18

TheGupta2012 requested a review from ryanhill1 December 11, 2024 10:13

ryanhill1 requested changes Dec 11, 2024

View reviewed changes

TheGupta2012 added 2 commits December 13, 2024 16:46

add more cov

5764f52

Merge branch 'main' into pyqasm-integration

5ba792c

Merge branch 'main' into pyqasm-integration

3f2b7a7

ryanhill1 approved these changes Dec 13, 2024

View reviewed changes

ryanhill1 merged commit e7b34b5 into main Dec 13, 2024

TheGupta2012 mentioned this pull request Dec 14, 2024

[FEATURE] Add custom include file support qBraid/pyqasm#113

Closed

Conversation

TheGupta2012 commented Oct 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Changes to the qbraid.passes.qasm

Changes to the qbraid.programs

Updates to coverage for transpiler

Uh oh!

github-actions bot commented Oct 24, 2024

Uh oh!

TheGupta2012 commented Nov 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryanhill1 Dec 3, 2024

Choose a reason for hiding this comment

Uh oh!

TheGupta2012 Dec 4, 2024 • 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!

ryanhill1 Dec 11, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ryanhill1 left a comment

Choose a reason for hiding this comment

Uh oh!

ryanhill1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ryanhill1 Dec 11, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ryanhill1 commented Dec 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheGupta2012 commented Dec 12, 2024

Uh oh!

TheGupta2012 commented Dec 13, 2024

Uh oh!

TheGupta2012 commented Dec 13, 2024

Uh oh!

ryanhill1 commented Dec 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TheGupta2012 commented Oct 24, 2024 •

edited

Loading

Changes to the `qbraid.passes.qasm`

Changes to the `qbraid.programs`

TheGupta2012 commented Nov 18, 2024 •

edited

Loading

TheGupta2012 Dec 4, 2024 •

edited

Loading

codecov bot commented Dec 5, 2024 •

edited

Loading

ryanhill1 commented Dec 11, 2024 •

edited

Loading