support reset and measure in qasmparser - #402
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for reset and measure instructions in the OpenQASM parser, addressing issues #192 and #345. The implementation introduces a new Reset gate class and extends the parser to handle both single-qubit and register-broadcast forms of these instructions.
Changes:
- Added
Resetgate class that models reset as post-selection followed by state preparation in ZX-diagrams - Extended QASM parser to handle
resetinstruction for single qubits and entire registers - Enhanced
measureinstruction to support register broadcast syntax (measure q -> c) - Added classical register tracking and automatic declaration in QASM output
- Implemented graph conversion logic to handle reset as effect + state vertices
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pyzx/circuit/gates.py | Added Reset gate class with to_qasm, reposition, and equality methods; added to_qasm method for Measurement |
| pyzx/circuit/qasmparser.py | Added parsing for reset and register-broadcast measure; added cregisters tracking |
| pyzx/circuit/graphparser.py | Added circuit_to_graph logic for Reset (effect+state vertices); added graph_to_circuit detection of Reset vs InitAncilla |
| pyzx/circuit/init.py | Added automatic classical register declaration inference from Measurement gates |
| tests/test_qasm.py | Added comprehensive tests for reset and measure including QASM round-trips and graph conversions |
| tests/test_init_postselect.py | Added unit tests for Reset gate functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
48b3cc3 to
6e7f0f1
Compare
6e7f0f1 to
dc5e02b
Compare
|
This looks great, thanks! |
|
Yes, looking at the official documention of openqasm, discarding is indeed the right semantics: https://openqasm.com/versions/3.0/language/insts.html |
|
You're right. The |
0a31e54 to
28a58cc
Compare
|
Based on suggestions in tqec/tqec#708, I added a test ( |
53fc11a to
15a9496
Compare
|
Is this now ready to merge? |
15a9496 to
7911198
Compare
|
I was hoping for more comments from the TQEC folks first, in case they had something to add, but it's ready to merge now. I think we can just merge this to move things along. It looks like you already have a volunteer to update the AllFeatures notebook in #374. |
|
Hi @dlyongemallo, I'm in the TQEC group, thank you for implementing this!! I was able to parse the HTHTHT... test circuit into ZX graph and also simplify it. However, both Nodes from measurement gates have phase I was able to bypass the rule by customizing it rule to allow brackets. Then, |
|
A circuit with measurements but no feedforward corrections represents a non-deterministic computation, so the lack of gFlow is expected. The test circuits added in this PR omits them because they haven't been implemented yet; that's PR #403. After that PR is merged, the gFlow problem should go away. Can you tell me a bit more about where Lark comes in? I don't quite understand how you triggered the error. |
Test interop with Stim for: surface code, repetition code, color code. QASM 2 with skip_dets_and_obs=True is the primary interop path.
7911198 to
5935ba1
Compare
|
Oh, never mind, I got it. All that's actually needed is a small change to |
|
@jvdwetering I think this PR is ready to merge. @KabirDubey Can you test if #403 fixes your gFlow problem? You can leave a comment in that PR if you have any issues. Thanks. |
|
@jbolns Are you calling Originally, the "ground-based representation" was the only way to convert measurements to a graph. PR #348 (by @Zhaoyilunnn) effectively renamed the original I'm not sure if anyone is actually using the @Zhaoyilunnn Did you keep the legacy @jvdwetering If it's not being used and was kept "just in case", can |
|
I used the standard Qiskit -> QASM -> PyZX (with c.to_graph()) pathway that I imagine most users would use. I wanted to check the full sequence could be realised. It can, which is very good. . To be on the safe side, I just checked using the test you added in the separate commit. Result is indeed equal. To be clear, I am actually asking rather than a suggestion framed as a question (hate it when people do that, actually: if one has a suggestion, one makes a suggestion). I do think the lonely spider seems unnecessary. Having said that, the other spiders I am indeed only asking about. Truth be said, they might even be needed for the actual computation (everything but the lone spider) to reduce nicely. |
|
I'm not sure what to do with the ground features. They were added a long time ago, but weren't really documented and nobody really uses them as far as I know, but it is a natural way to represent mixed processes in ZX-diagrams. My personal opinion is that using symbolic variables for representing indeterminancy is better for most cases, but I coul definitely see it being useful for some people to have the ground representation. So if it is not hurting anyone, then keeping it would be fine by me. Someone would really need to document really well how to deal with non-determinism in pyzx at some point though. |
Seems that Wetering prefer to keep it. At my side, I didn't keep it for a particular reason |
|
@Zhaoyilunnn Tangentially related to this issue: are you one of the maintainers of QASMBench? I'm using it to benchmark my qiskit-zx-transpiler, and the implementation of |
For what it's worth, when I tested your parser, I sourced benchmarks from FTCircuitBench. I think it's slightly more likely to be maintained because it's more recent but it may not have the circuits you're looking for. |
Sorry about the late response, unfortunately I am not a maintainer. As far as I know, you may reach out to https://github.com/uuudown |




Fixes #192. Relates to #345.