Skip to content
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 native PauliRot implementation in LightningQubit #834

Merged
merged 67 commits into from
Aug 19, 2024
Merged

Conversation

vincentmr
Copy link
Contributor

@vincentmr vincentmr commented Aug 1, 2024

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 the
    change, 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:
Pauli rotations come up in many places, and importantly in the time evolution of qchem Hamiltonians. It is therefore worth considering ways to accelerate their execution.

Description of the Change:
Implement applyPauliRot in the LM gate kernel. Invoke applyPauliRot directly from the SV class and add bindings to the Python layer. Add custom decomposition rule for PauliRot operations.

Benefits:
Faster Pauli rotations. I performed a benchmark on random PauliRotations (runtime > 1.0 sec and at least 5 of them) through the Python layer. The data remains noisy with 5 samples because the performance varies depending on the specific "XYZ" sequence (which translates into more or less predictable memory access patterns). Overall, we see an advantage for 3+ qubits and up.

speedup_vs_ntargets

The speed-ups are also good when parallelizing with OpenMP threads (both cases using 16 threads)

speedup_vs_ntargets_omp16

After rewriting the PauliRot gates as one-qubit gates of sort, writing AVX kernels for PauliRot may be worth it.

For an entire workflow like time-evolving a QChem Hamiltonian, the gains are slower coming because PauliRot is not necessarily the main bottleneck at all scales, but it is once the molecules are big enough. This is shown in the speed-up bar graph below.

speedup_vs_mol

Possible Drawbacks:

Related GitHub Issues:
[sc-69801]
[sc-71642]

Copy link

codecov bot commented Aug 2, 2024

Codecov Report

Attention: Patch coverage is 23.07692% with 10 lines in your changes missing coverage. Please review.

Project coverage is 92.39%. Comparing base (6b42030) to head (e19e21e).
Report is 1 commits behind head on master.

Files Patch % Lines
...nnylane_lightning/lightning_qubit/_state_vector.py 0.00% 6 Missing ⚠️
...ylane_lightning/lightning_qubit/lightning_qubit.py 33.33% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #834      +/-   ##
==========================================
- Coverage   95.17%   92.39%   -2.79%     
==========================================
  Files         113       70      -43     
  Lines       18141    11250    -6891     
==========================================
- Hits        17265    10394    -6871     
+ Misses        876      856      -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vincentmr vincentmr marked this pull request as ready for review August 2, 2024 20:06
@vincentmr vincentmr marked this pull request as draft August 6, 2024 14:36
@vincentmr vincentmr marked this pull request as ready for review August 15, 2024 17:02
@vincentmr vincentmr requested review from LuisAlfredoNu, mlxd and a team August 15, 2024 17:03
vincentmr added a commit that referenced this pull request Aug 19, 2024
### 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`](../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`.

- [x] Add a new entry to the `.github/CHANGELOG.md` file, summarizing
the
      change, 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:**
@mlxd
[mentioned](#834 (comment))
that "the use of structured bindings/lambdas when combined with OpenMP
often fails to compile with clang due to missing support"

**Description of the Change:**
Replace structured bindings by variables in `GateImplementationsLM.hpp`.

**Benefits:**
Compile with Clang & OpenMP.

**Possible Drawbacks:**

**Related GitHub Issues:**

---------

Co-authored-by: ringo-but-quantum <github-ringo-but-quantum@xanadu.ai>
Copy link
Member

@multiphaseCFD multiphaseCFD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank @vincentmr! Nice work! Just a few comments.

Copy link
Contributor

@LuisAlfredoNu LuisAlfredoNu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM just a couple of comments 🚀

vincentmr and others added 2 commits August 19, 2024 13:48
…cpu_kernels/GateImplementationsLM.hpp

Co-authored-by: Luis Alfredo Nuñez Meneses <alfredo.nunez@xanadu.ai>
Copy link
Member

@multiphaseCFD multiphaseCFD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @vincentmr for the nice work.

Copy link
Contributor

@LuisAlfredoNu LuisAlfredoNu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work and good improvement for Pauli Rotations 👍
Thank you @vincentmr

@vincentmr vincentmr merged commit 83ac9d8 into master Aug 19, 2024
95 of 97 checks passed
@vincentmr vincentmr deleted the lq_pauli_rot branch August 19, 2024 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants