-
Notifications
You must be signed in to change notification settings - Fork 39
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
Add GlobalPhase to all devices #579
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #579 +/- ##
==========================================
- Coverage 98.96% 98.47% -0.49%
==========================================
Files 169 169
Lines 24308 24387 +79
==========================================
- Hits 24056 24015 -41
- Misses 252 372 +120 ☔ View full report in Codecov by Sentry. |
… much memory for large nq [skip ci].
Grover algorithm returns wrong results with [sc-51671] |
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 @vincentmr
I think we can come back and discuss this in a few weeks
pennylane_lightning/core/src/simulators/lightning_gpu/initSV.cu
Outdated
Show resolved
Hide resolved
pennylane_lightning/core/src/simulators/lightning_gpu/initSV.cu
Outdated
Show resolved
Hide resolved
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #579 +/- ##
==========================================
+ Coverage 98.49% 98.72% +0.22%
==========================================
Files 169 169
Lines 24717 24557 -160
==========================================
- Hits 24345 24243 -102
+ Misses 372 314 -58 ☔ 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.
Hi @vincentmr, I left a few comments.
...ightning/core/src/simulators/lightning_gpu/gates/tests/Test_StateVectorCudaManaged_Param.cpp
Outdated
Show resolved
Hide resolved
...lightning/core/src/simulators/lightning_qubit/gates/tests/Test_GateImplementations_Param.cpp
Outdated
Show resolved
Hide resolved
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 @vincentmr
Just a few final questions, and will be happy to approve once they are commented on.
Before submitting
Please complete the following checklist when submitting a PR:
All new features must include a unit test.
If you've fixed a bug or added code that should be tested, add a test to the
tests
directory!All new functions and code must be clearly commented and documented.
If you do make documentation changes, make sure that the docs build and
render correctly by running
make docs
.Ensure that the test suite passes, by running
make test
.Add a new entry to the
.github/CHANGELOG.md
file, summarizing thechange, and including a link back to the PR.
Ensure that code is properly formatted by running
make format
.When all the above are checked, delete everything above the dashed
line and fill in the pull request template.
Context:
Prior to #516 , the Lightning devices could not properly run Grover's algorithm (among others) because
GlobalPhase
andC(GlobalPhase)
are not supported. This would also be true of any circuit which ends up withGlobalPhase
orC(GlobalPhase)
after decomposition. In addition, L-GPU's custom gate cache was not passed a parameter to distinguish between gates. This meant, for example, thatMultiControlledX([0, 1, 2], "111")
andMultiControlledX([0, 2], "00")
were applied as the same operation. This likely affected any circuit with more than one version ofDescription of the Change:
Add
GlobalPhase
andC(GlobalPhase)
support in L-GPU and L-Kokkos. For the sake of time,C(GlobalPhase)
is implemented by generating the diagonal entries withglobal_phase_diagonal
at the Python layer, and then passing it through the gate matrix argument down to the C++ layer and adding a special branch for it inapplyOperation
. To fix the gate cache, we pass the gate hash as a parameter when applying any of the above gates to make sure the gate cache adds a new matrix.Benefits:
Bug fix, can now run Grover with L-GPU and L-Kokkos.
Possible Drawbacks:
Higher memory demand in the gate cache.
Related GitHub Issues: