-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adding new single-qubit gates #333
Conversation
The gates fail this test (hope this is not too bad), The gate pass all the other tests for single-qubit gates.
|
I would expect there to be a but in the gates if they fail this test. |
Indeed, some part of implementation is wrong. My bad! |
Indeed, the implementation was so bad. In particular, The macro qubit kernel descriptions were wrong that's why the failing tests occured in this testset "Convert between small ops". The build failed because of more occasional (comparison to MC under perturbative expansion purification example testset) error not related to this PR. The windows latest CI fails because of this error:
|
BTW, A small comment that is not related to this PR: QuantumClifford.jl/src/QuantumClifford.jl Line 78 in 263195c
There is no |
should be here QuantumClifford.jl/src/project_trace_reset.jl Line 887 in 263195c
|
Thanks, It seems I don't have this in the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #333 +/- ##
==========================================
+ Coverage 82.41% 82.75% +0.34%
==========================================
Files 62 62
Lines 4162 4308 +146
==========================================
+ Hits 3430 3565 +135
- Misses 732 743 +11 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good. Thank you for spending time on it! I think the only thing left is:
- thinking through whether there is some more idiomatic naming convention for us compared to the
DAG
suffix. Probably we will just stick to that suffix. - Having a set of tests that specifically check that gates match the definitions that stim uses. E.g. something like
CliffordOperator(gate) == C"Z X"
or whatever the appropriate C string is, taken from the markdown file you shared.
For us, Please share your thoughts on this. Thank you! |
Thanks again for the very hepful |
Thanks, this is very useful! |
This PR aims to add the missing single qubit gate w.r.t stim. Added and Tested all the missiing single qubit gates that stim has: https://github.com/quantumlib/Stim/blob/main/doc/gates.md
In this book(https://threeplusone.com/pubs/on_gates.pdf),
SQRTX
is calledV
gate, soV
gate is used as its name.