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 CUDA-12 capability #477

Merged
merged 17 commits into from
Aug 25, 2023
Merged

Add CUDA-12 capability #477

merged 17 commits into from
Aug 25, 2023

Conversation

vincentmr
Copy link
Contributor

@vincentmr vincentmr commented Aug 21, 2023

Add CUDA-12 capability

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:
The Kokkos backend could not be built or executed on devices. A few minors fixes are necessary. Notably, JacobianData works with a data pointer which is not accessible if the data lives on a device.

Description of the Change:
Fix few bugs, and get the state vector data from the device to the host, into JacobianData.

Benefits:
This code builds with CUDA-12 and executes on capable NVidia devices.

Possible Drawbacks:
This is a quick fix, but we should think about a more perennial solution. For example, we could store the state vector in JacobianData or pass the state vector to adjointJacobian as follows adjointJacobian(std::span{jac}, jd, sv). sv could be [[maybe_unused]] in LQubit and used in device-executing backends.

Related GitHub Issues:

…ata` to work with devices.

M  pennylane_lightning/core/src/simulators/lightning_kokkos/StateVectorKokkos.hpp; `applyMatrix` bugfix: use intermediate hostview to copy matrix data; same bugfix for `getDataVector`.
M  pennylane_lightning/core/src/simulators/lightning_kokkos/algorithms/AdjointJacobianKokkos.hpp; use copy constructor.
M  pennylane_lightning/core/src/simulators/lightning_kokkos/measurements/MeasurementsKokkos.hpp; use copy constructor.
M  pennylane_lightning/core/src/simulators/lightning_kokkos/observables/ObservablesKokkos.hpp; use copy constructor.
M  requirements-dev.txt; add clang-format-14.
@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@vincentmr vincentmr changed the title M pennylane_lightning/core/src/bindings/Bindings.hpp; hack `Jacobian… Add CUDA-12 capability Aug 21, 2023
@vincentmr vincentmr marked this pull request as ready for review August 21, 2023 18:21
@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #477 (fb82dea) into master (e7922ff) will decrease coverage by 0.39%.
The diff coverage is 91.20%.

@@            Coverage Diff             @@
##           master     #477      +/-   ##
==========================================
- Coverage   97.22%   96.84%   -0.39%     
==========================================
  Files         142      142              
  Lines       16420    16275     -145     
==========================================
- Hits        15964    15761     -203     
- Misses        456      514      +58     
Files Changed Coverage Δ
...htning/core/src/algorithms/AdjointJacobianBase.hpp 100.00% <ø> (ø)
...s/lightning_kokkos/measurements/ExpValFunctors.hpp 56.93% <ø> (-43.07%) ⬇️
...asurements/tests/Test_StateVectorKokkos_Expval.cpp 100.00% <ø> (ø)
...ghtning_qubit/algorithms/AdjointJacobianLQubit.hpp 96.25% <ø> (ø)
...ghtning_kokkos/measurements/MeasurementsKokkos.hpp 94.47% <85.45%> (+0.11%) ⬆️
pennylane_lightning/core/_version.py 100.00% <100.00%> (ø)
...core/src/algorithms/tests/Test_AdjointJacobian.cpp 100.00% <100.00%> (ø)
.../simulators/lightning_kokkos/StateVectorKokkos.hpp 93.76% <100.00%> (-0.24%) ⬇️
...htning_kokkos/algorithms/AdjointJacobianKokkos.hpp 98.07% <100.00%> (-0.08%) ⬇️
...rs/lightning_kokkos/gates/GateFunctorsNonparam.hpp 84.94% <100.00%> (+0.05%) ⬆️
... and 3 more

Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Happy to see this GPU fix go in.

.github/CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@AmintorDusko AmintorDusko left a comment

Choose a reason for hiding this comment

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

Thank you for that!

@AmintorDusko
Copy link
Contributor

Hey @vincentmr, you may need to re-trigger CIs here.

* Introduce std::unordered_map<std::string, ExpValFunc> expval_funcs_.

* Introduce applyExpectationValueFunctor.

* Add binding to LKokkos expval(matrix, wires). Combine expval functor calls into two templated methods. Call specialized expval methods when possible. Remove obsolete 'Apply directly' tests.

* Update changelog.

* Add test for arbitrary expval(Hermitian).

* Add getExpectationValueMultiQubitOpFunctor.

* Add typename hint for macos.

* Add typename macos.

* Use Kokkos::ThreadVectorRange policy for innerloop in getExpectationValueMultiQubitOpFunctor.

* Couple fix for HIP.

* Use inner product scheme instead of getExpectationValueMultiQubitOpFunctor to compute multi-qubit expval.
@vincentmr vincentmr requested a review from mlxd August 25, 2023 15:05
@vincentmr vincentmr merged commit 1bd68b4 into master Aug 25, 2023
58 of 60 checks passed
@vincentmr vincentmr deleted the bugfix/cuda12 branch August 25, 2023 17:22
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.

3 participants