TerminalLogicalMeasurement implementation #627
Conversation
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
david-pl
left a comment
david-pl
left a comment
There was a problem hiding this comment.
Generally, the code looks good. Just some small comments throughout. Also:
- Please add a test.
- What's the plan for method table impls for the new statement? I suppose this should work with the simulator and similar things. Is there a follow-up issue?
weinbe58
left a comment
weinbe58
left a comment
There was a problem hiding this comment.
If @david-pl is OK with a breaking change I think we should probably go ahead and rename the groups but otherwise @johnzl-777 please roll that change back.
Overall it looks good I have no other changes to propose
|
@david-pl @johnzl-777 No, I'm fine with the breaking change. I was just wondering. |
|
@david-pl so if we have simulator support for this, does that mean under the hood we're keeping track of the physical qubits? (hope this isn't too silly a question) |
No simulator support for this dialect. It doesn't make sense here because you're converting from logical qubit to physical qubits. |
|
Trying to add a test to the validation, seems to be some obnoxious circular import thing going on. Will merge once I fix it (or request another review if it's a large change) |
|
@johnzl-777 I think that's fine. Consider it approved from my side, but I'm not sure whether @weinbe58 has any comments. |
|
@weinbe58 gave a thumbs up on the comment and hasn't revoked/changed his original approval so I think we're good (: |
|
@david-pl With the docstrings checking feature in our CI will it always fail until we get everything fixed? |
|
@johnzl-777 yes. I know, not ideal, but I've yet to figure out how to make it check changes only. FWIW, you can check your files with |
I believe the signature should be in line with what was requested in #609 but I went ahead and made a dedicated dialect for the additional statement.
I originally wanted to follow something like
native's structure which has its own version of the squinqubitdialect (same name but tucked away elsewhere) although I wasn't sure if this was something we wanted to mirror.