-
Notifications
You must be signed in to change notification settings - Fork 40
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
Fixes #975 : Implement QFT in lightning.qubit and benchmark #990
base: master
Are you sure you want to change the base?
Conversation
@gzquse I just triggered the CI |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #990 +/- ##
===========================================
- Coverage 95.47% 38.94% -56.53%
===========================================
Files 221 43 -178
Lines 33055 2963 -30092
===========================================
- Hits 31559 1154 -30405
- Misses 1496 1809 +313 ☔ View full report in Codecov by Sentry. |
review required |
Thanks @gzquse |
Hi @tomlqc , You are right. I have some issues using a local apple chip to do the benchmark and I also tried to use our uni HPC to do the experiment but seems the CMake is not the same. May I ask if it is possible to get a test environment from Pennylane's HPC? Therefore, I can test it a bit further. |
Benchmark Jupyter notebook for reference
|
Hi @gzquse, |
tests/test_gates.py
Outdated
|
||
@pytest.mark.parametrize("num_qubits", [2, 3, 4]) | ||
def test_qft_gate(num_qubits): | ||
dev = qml.device("lightning.qubit", wires=num_qubits) |
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.
Just a note before we start the actual review. Please use default.qubit
in this pytest to check the validity of the new implementation.
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.
Does the latest change test your implementation against default.qubit
? This is what I meant.
You may want to take a look at the other tests in this file.
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.
yes
Gist |
https://www.google.com/search?q=how+to+link+github+pr+to+issue |
Is your laptop already struggling with >10 qubits? |
Hi @tomlqc, |
I assume the original pr was under the forked branch. So the "Fix" does not work? |
Thanks @gzquse for the update
|
I added the line to the description (not the title) and it worked fine. |
Thanks for pointing out |
Hi,
|
Thanks @gzquse |
Dear Thomas,
Solution from my side
Add the Struct: Add a new specialization of the GateOpToMemberFuncPtr template for the QFT gate.
Implement member
Use PENNYLANE_RUN_TEST Macro, implement test function, and add test case
QFT(∣000⟩)= 1/sqrt(8) * (∣000⟩+∣001⟩+∣010⟩+∣011⟩+∣100⟩+∣101⟩+∣110⟩+∣111⟩)
so each amplitude is approximately 0.353553
Fixes #975