-
Notifications
You must be signed in to change notification settings - Fork 50
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
[non-Clifford] In-place Pauli measurements (projectrand!
) for GeneralizedStabilizer
with expanded test suite
#427
base: nonclif
Are you sure you want to change the base?
Conversation
Please help review this PR, @Krastanov, thank you! |
projectrand!
) for GeneralizedStabilizer
Hi, @Krastanov! Just a friendly bump on this. Thanks! |
projectrand!
) for GeneralizedStabilizer
projectrand!
) for GeneralizedStabilizer
with expanded test suite
… qubits) - from 60s to <1s now
Thanks for continuing with this. I have not really done much of an in-depth review besides skimming through the new tests to make sure they are not too heavy on the CI system (hence the commit). The same example that I used on your previous PR still seem to give some issues, but they seem to be API misunderstandings, not correctness issues:
The normalization question is a bit difficult to design around -- there are certainly situations where an unnormalized result is preferred. But we also need to keep consistency with pre-existing API.
So I guess we can pick any convention we want here... And the closest one to the currently existing So how about the following:
I posted the API design issue to the QuantumInterface repo qojulia/QuantumInterface.jl#45 |
Fixed the convention. Thanks! Originally, in the
This was Ted's design choice/convention. So, I didn't poke his convention at all. Yeah, let's call it Thank you for the API suggestions. I thought it would be better to discuss the API stuff than to do anything beforehand, so I didn't touch anything API related (didn't added anything irrelevant). |
CI related message: The performance tracking CI error is likely due to
|
Thank you for the API suggestions! I have incorporated the three tasks that were suggested. When the invsparsity is 1, we do normalization. This helps not mess up the behavior of
Please help review this PR, thank you! |
This PR implements the original approach for the Pauli Measurements algorithm as described in Ted’s paper. The algorithm is in-place and closely translates the algorithm/proof outlined on page 8 of the paper. It incorporates the details from pages 6 to 8 and equations (14) through (26) to fully capture the aspects of working with this weird stabilizer basis given in the paper.
Previously, I had considered this approach but avoided it due to some confusion, which led me to overlook the method explicitly given on page 8. The manual approach implemented in #355 missed some details of the algorithm in the special stabilizer basis constructed by Ted. In retrospect, it feels like manually building the wall brick by brick, even though the blueprint for the wall was already provided in Algorithm 2.
Thoughts: The approach in #355 relied on
expect
to advance the computation. However, this method diverges from the original algorithm described in the paper, especially in theelse
case of the algorithm. 2) The original algorithm includes several parameters and functionalities that were not fully captured. This PR aims to rectify that by directly implementing the specific details and behaviors as outlined in the paper.Thanks so much for your feedback. I now realize that faithfully translating the paper into code requires revisiting and challenging both my initial assumptions and any new assumptions I may be making. Hopefully, I aim to get on the right track.
An initial demonstration shows that it addresses the initial pain points highlighted in #355. I am currently working on adding more tests to enhance its validation. In the
if
case, we don't need to update the stabilizer basis (equation 17), so I returnednothing
as second argument.