Add unit tests for PauliStringLCU block encoding - #94
Conversation
|
I found that my contraction of the select bits shouldn't have been necessary - it was only needed to work around a bug in Qualtran 0.4.0. I've backported a fix from 0.5.0 and that allows me to simplify the tests nicely. |
BrendanKKrueger
left a comment
There was a problem hiding this comment.
I saw that John sent you some feedback. Since he understands this topic better than I do, I'll just assume that he's handling the review for this merge request unless informed otherwise.
|
[Review notes from @johngolden's email] I looked at your code for the recent PR and found some issues. The main one is that you pass sqrt(coeffs) to StatePreparationAliasSampling when that wants abs(coeffs). You can see in pyLIQTR's own PauliStringLCU that they keep those as two separate variables (the sqrt one is only for the prepare_type=None branch). And then your tests all use equal coefficients, which makes the issue invisible, since sqrt of equal values is still equal. If you make them unequal you'll also need to drop probability_eps to 0.1, otherwise the correct code fails too because the aliasing is too coarse. I've attached a .py (masquerading as a .txt) which shows this. Should be a quick change to the code and the tests to get up and running. A few other things I noticed:
|
8490fec to
3b40617
Compare
This adds two unit tests for the (Qualtran-based) PauliStringLCU block encoding, which verify that the upper corner of the generated block encoding is equal to the original Hamiltonian, up to a scale factor. It also does some cleanup of the PauliStringLCU logic and fixes a minor bug.
EDIT: I've also added the same two tests for the PyLIQTR version of the encoding.