-
Notifications
You must be signed in to change notification settings - Fork 49
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
noncliff: enhanced error message for inapplicable Project! of GeneralizedStabilizer #416
Conversation
a87be4c
to
393e59a
Compare
393e59a
to
fdfbf38
Compare
4ddd241
to
ba1d34a
Compare
The PR is ready for review. Please run it on the console to check if any further formatting changes are required. Thank you! |
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.
looks good! I will do some minor rewording on the next round of review, but first check out below for how this can be packaged a bit more neatly into Julia's error handling mechanism.
src/nonclifford.jl
Outdated
@@ -172,15 +172,41 @@ function _allthreesumtozero(a,b,c) | |||
true | |||
end | |||
|
|||
function project!(sm::GeneralizedStabilizer, p::PauliOperator) | |||
|
|||
project!(::GeneralizedStabilizer, ::PauliOperator) = |
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 can be done a bit more neatly using MethodError
and a registered error hint.
E.g. throw throw(MethodError(f, (sm,p)))
where sm and p are the arguments to this function.
Then use register_error_hint
to register the additional explanation message: https://docs.julialang.org/en/v1.12-dev/base/base/#Base.Experimental.register_error_hint
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.
Thanks so much for the suggestion!
marking as draft to make it easer for me to track what needs review |
c26c4c0
to
a8a8589
Compare
Thank you! I have incorporated your comments and checked the output on console. Thank you for rewording as you see fit! Edit: Fixed some minor doctests test errors in recent commits |
… would print for things like `project!(1)`) and guard against this experimental feature being dropped
Thank you for polishing the PR! |
I have incorporated the relevant details from the paper to create an informed error message. This PR is inspired from this comment: #355 (comment)
I think we only need one
proj,
so removed_proj₋
or_proj₊
in favor ofproj
.