unitaryfund / qrack

Comprehensive, GPU accelerated framework for developing universal virtual quantum processors
https://qrack.readthedocs.io/en/latest/
GNU Lesser General Public License v3.0
174 stars 38 forks source link

Add `PhaseRootNMask` to `QInterface`, implement for `QEngineCPU` #1009

Closed jpacold closed 3 months ago

jpacold commented 3 months ago

Draft PR to start addressing #876 -- I would appreciate feedback about whether I'm on the right track.

Reading the issue description, I think the intended behavior of PhaseRootNMask(N, m) is:

$$ \left|x \right> \rightarrow \exp( qi\pi /2^{N-1} ) \left|x \right> $$

where q = popcount(x & m) or equivalently q = popcount(x & m) % (1 << N) . So far I have only added implementations to QInterface and QEngineCPU.

One thing I'm confused about: the base implementation of QInterface::PhaseRootN calculates a phase factor of pow(-ONE_CMPLX, (real1)(ONE_R1 / pow2Ocl(n - 1U))). This gives $\exp(-i\pi / 2^{n-1})$ where I would expect $\exp(+i\pi / 2^{n-1})$. As a result the test that I added currently doesn't pass.

WrathfulSpatula commented 3 months ago

Thanks for the PR! You are on the right track. Yes, this is equivalent to applying a phase factor of $(-1)^{1/2^{N-1}}$ on every bit in |1> state.

I'll clarify that the reason for the sign convention is so the form of the phase factor will be $(-1)^{1/2^{N-1}}$. So, the point is that the base is $-1$, and this is a root rather than a power. Your convention would make more sense for "PhasePowerN()," as a method name, if you see my point.

Let me know if I can help with the OpenCL side of the implementation or any further questions!

jpacold commented 3 months ago

Thanks, I changed the sign convention. I will definitely need help with OpenCL since this is the first time I've seen it, but I think the earliest I can revisit this will be Thursday or Friday night.

WrathfulSpatula commented 3 months ago

@jpacold, please comment directly on #876 so I can assign you and close the issue before the end of the event. You have a CPU-based implementation, at least, and you deserve the bounty.

jpacold commented 3 months ago

Thanks, much appreciated. 😃 Should I open another issue for the OpenCL implementation or just open another PR when I have something?

jpacold commented 3 months ago

I finally got a chance to get back to this and did the following:

WrathfulSpatula commented 3 months ago

By the way, don't freak about the test failure: it's a very rare stochastic failure that has nothing to do with your work, and I've seen it before. Let's merge! Thanks again!

jpacold commented 3 months ago

You're welcome and thanks for taking over from here. After seeing all the Python/Rust projects out there it was great to find a mainly C++ issue to work on!