Conversation
After in-place dialect conversion, the converted method was still registered as a caller of its callees. Re-decorating a callee gate (e.g. re-running a notebook cell) triggered recompile_callers, which tried to re-verify the now-squin IR with stale qasm3 passes, failing with "expected Qubit, got AnyType" on Measure statements. Clear callee backedges and null out run_passes after conversion.
|
Hi bloqade-circuit maintainer, there are still few features I'd like to add. But this PR getting bigger. So I think it makes sense to first get a review on the structure of the qasm3 module, and I can add more features as follow-up. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
david-pl
left a comment
david-pl
left a comment
There was a problem hiding this comment.
Hi @yitchen-tim and thank you very much for this contribution! Overall it looks great, the structure looks good. The only thing that really might need some changes is the emitter, I think (see comment below).
The rest is fine, I think.
there are still few features I'd like to add.
Do you mind sharing what features specifically you still plan on adding? Specifically, I'm wondering whether you plan on adding control flow?
Also, apologies if some of my comments fall into the category of features you plan to add anyway.
The CI is actually passing, there are just two types of failures:
- Some permission errors when trying to upload the code coverage (since you're coming from a fork). Ignore those.
- Formatting and linter errors. Pylance should be good at catching type issues. For formatting you could consider installing the pre-commit hooks as mentioned here to catch them before committing: https://bloqade.quera.com/latest/contrib/
But you can also ignore them for now and I can make sure the linter is happy once we decide to merge the PR.
Another thing to keep in mind: it seems to me that quite a bit of functionality could be shared between qasm2 and qasm3 (e.g. the expr dialect, types). However, this might need some breaking changes in qasm2. Besides, I feel like this is beyond the scope of this PR. For now, let's just keep things separate, but something to keep in mind for later.
|
Hi @david-pl thanks for the comments. I have made updates to address the comments. I got the linting done with pre-commit. The linting update the code beyond my changes, hopefully that's okay. When I ran unit tests locally, there are some failed tests that is not related to this PR. I didn't try to fix them.
I agree keeping them separate makes sense for now. Given that QASM2 has stopped developing while QASM3 is still evolving, I would think the development should center around QASM3. QASM2 should support strictly a subset of QASM3, so I can see some de-duplicate we can do. For example, QASM3 becomes a combination of QASM2 dialects + few new dialect. But right now, the QASM2 dialect isn't defined purely with the spec. It also intertwines with features like schedule, parallel and AddressAnalysis. It's a bit not obvious for me what is a clean way to inherit dialects from QASM2.
These are what I have in mind. I might not necessarily do all of them, but they are what I can think of for now.
|
|
@yitchen-tim thanks for the response! I agree with all your points. The CI failure is strange: it failed locally for me once, but appears to consistently pass for now. I'll need to investigate further. Other than that, the emitter now looks clean, thanks. Overall the structure is good! The list of features you mention is quite extensive. Feel free to defer some of them to follow-up issues if you think it's too much for this one PR. |
david-pl
left a comment
david-pl
left a comment
There was a problem hiding this comment.
@yitchen-tim I fixed the issue with the typos hook. There's still a bug here that causes the tests to fail, but only sometimes. The issue is that there are two different methods for GateFunction in the emit, see below.
…emit rewrite pass
|
@david-pl thanks for taking a look. I cleaned up code (remove unnecessary overwriting of parent methods, remove dup registration of GateFunction etc). I also improved test coverage. The two remaining failed checks are (1) CI post, which you mentioned it's due to permission issue, and (2) docstrings which needs fix in a lot of code in other modules so I just leave it. Regarding other features in my wishlist. I agree with you that we defer. I don't plan to add more feature to this PR as it is already very bulky. |
|
@yitchen-tim okay, great. Thank you for this contribution! I will take another look next week to make sure it's ready to be merged. But it already looks very good!
Sure thing! |
2 participants
Add OpenQASM 3.0 support
Implements the OpenQASM 3.0 (QASM3) workflow including parsing, IR dialects, emission, and conversion to the Squin dialect.
What's new
bloqade.qasm3module with dialect definitions for core statements, unitary ops, and expressionsopenqasm3+larkbacked)loads/loadfilehelpers and a@qasm3.main/@qasm3.gatedecorator experience mirroring the existing QASM2 APIqasm3.emit) that serialises the IR back to valid QASM3 source, with optionalincludefile supportQASM3ToSquinpass that rewrites a QASM3 kernel in-place to the Squin dialect, including argument-order fixups for rotation gates (RX,RY,RZ,UGate)run_passesare cleared after conversion so re-decorating a callee in a notebook doesn't trigger a type errorqasm3optional dependency group (openqasm3[all]~=1.0.1,lark>=1.2.2)Tests
Comprehensive test suite covering AST construction, round-trip parsing, emission, lowering, and the end-to-end QASM3→Squin pass. Sample
.qasmprograms included for bell state, rotations, resets, and mixed multi-qubit circuits.