Add Catalyst specific Lightning Kokkos class #770

AmintorDusko · 2024-06-19T12:43:14Z

Context: Catalyst needs to build a class wrapping the Lightning Kokkos class. Here we are moving the logic from Catalyst to the Lightning repository.

Description of the Change: Moving code from Catalyst to this repository and updating the build system to build against Catalyst.

Benefits: Catalyst wheels will build faster.

Possible Drawbacks: Our build system now relies on headers coming from Catalyst. It is unclear if this may cause a deadlock in the future because of our cyclic dependencies (PennyLane, PennyLane Lightning, Catalyst). Chances are small because we don't rely on Catalyst wheels or build it from scratch.

Usage: This PR offers three ways to configure the CMake building system:

Providing a local source path for Catalyst. Example:

cmake   -BBuildTests -G Ninja \
        -DCMAKE_INSTALL_PREFIX=$path_to_kokkos \
        -DCATALYST_SRC_PATH=$path_to_catalyst \
        -DCMAKE_BUILD_TYPE=Debug \
        -DBUILD_TESTS=ON \
        -DENABLE_WARNINGS=ON \
        -DPL_BACKEND=lightning_kokkos

Providing a GIT TAG. Example with the main branch:

cmake   -BBuildTests -G Ninja \
        -DCMAKE_INSTALL_PREFIX=$path_to_kokkos \
        -DCATALYST_GIT_TAG=main \
        -DCMAKE_BUILD_TYPE=Debug \
        -DBUILD_TESTS=ON \
        -DENABLE_WARNINGS=ON \
        -DPL_BACKEND=lightning_kokkos

If the dev/user doesn't say anything the configuration will default to get headers from Catalyst GitHub main branch. Example:

cmake   -BBuildTests -G Ninja \
        -DCMAKE_INSTALL_PREFIX=$path_to_kokkos \
        -DCMAKE_BUILD_TYPE=Debug \
        -DBUILD_TESTS=ON \
        -DENABLE_WARNINGS=ON \
        -DPL_BACKEND=lightning_kokkos

Check out ADR 76 for alternative approaches.

[sc-59312]

github-actions · 2024-06-19T12:43:28Z

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.

codecov · 2024-06-19T12:50:10Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.09%. Comparing base (91241a0) to head (ee1d8b5).
Report is 95 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (91241a0) and HEAD (ee1d8b5). Click for more details.

HEAD has 4 uploads less than BASE

Flag BASE (91241a0) HEAD (ee1d8b5)

10 6

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #770      +/-   ##
==========================================
- Coverage   94.32%   87.09%   -7.24%     
==========================================
  Files         109       74      -35     
  Lines       15783    11152    -4631     
==========================================
- Hits        14888     9713    -5175     
- Misses        895     1439     +544

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

AmintorDusko · 2024-06-19T12:50:45Z

Catalyst files were moved from here.
I only updated the copyright date, but we might want to return to this later and review these files.

Edit: Catalyst files also needed some format updates.

AmintorDusko · 2024-06-19T13:09:26Z

Tests are still needed.

vincentmr

Thanks @AmintorDusko , not much in the way of this PR, I wrote some thoughts for future work as suggestions.

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/CMakeLists.txt

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/LightningKokkosObsManager.hpp

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/LightningKokkosSimulator.cpp

…neAI/pennylane-lightning into add/rtd_lightning_kokkos

maliasadi

Thank you @AmintorDusko! 🎸

.github/CHANGELOG.md

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/CMakeLists.txt

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/LightningKokkosSimulator.cpp

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/LightningKokkosObsManager.hpp

mlxd

Thanks folks. I have a few questions, but given the size we can explicit backlog anything that's needed.

Though, are we expecting the Kokkos data to work with a CUDA backend, as well as an OpenMP? Given we are using std::vector of kokkos complex in some places, I wonder if this will cause issues.

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/CMakeLists.txt

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/LightningKokkosObsManager.hpp

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/LightningKokkosSimulator.cpp

...ghtning/core/src/simulators/lightning_kokkos/catalyst/tests/Test_LightningKokkosGradient.cpp

...htning/core/src/simulators/lightning_kokkos/catalyst/tests/Test_LightningKokkosSimulator.cpp

dime10

Since this essentially moves the files over there is not much for me to review, it is better if lightning maintainers give it a good review from their perspective. I've answered all the questions I could however.

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/CMakeLists.txt

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/LightningKokkosObsManager.hpp

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/LightningKokkosSimulator.cpp

...ghtning/core/src/simulators/lightning_kokkos/catalyst/tests/Test_LightningKokkosGradient.cpp

...htning/core/src/simulators/lightning_kokkos/catalyst/tests/Test_LightningKokkosSimulator.cpp

…neAI/pennylane-lightning into add/rtd_lightning_kokkos

mlxd

💯

vincentmr

Thanks @AmintorDusko , I just have a question about the change log.

.github/CHANGELOG.md

vincentmr

Massive PR! Looks good to me, thanks @AmintorDusko .

… We make a small update to track these changes. The Catalyst PR adds seeding to qjit: PennyLaneAI/catalyst#936

### Before submitting Please complete the following checklist when submitting a PR: - [x] 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! - [x] 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`. - [x] 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. - [x] 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 lightning kokkos files in the Catalyst repo has changed since #770. We make a small update to track these changes. The Catalyst PR that made the changes added seeding to qjit: PennyLaneAI/catalyst#936 **Description of the Change:** The `lightning_kokkos/catalyst` files now has the MCM seeding support added in catalyst PennyLaneAI/catalyst#936 **Benefits:** unblocks kokkos with catalyst **Possible Drawbacks:** None **Related GitHub Issues:** None --------- Co-authored-by: ringo-but-quantum <github-ringo-but-quantum@xanadu.ai>

AmintorDusko added 6 commits June 19, 2024 07:58

move catalyst files as is

c659ba5

update year

f1d99b3

add a CMakeLists file to build against local and GH catalyst TAG

b384b86

add the new catalyst directory to the build system

84036a4

Merge branch 'master' into add/rtd_lightning_kokkos

341e85f

not today Ringo

0f363c9

AmintorDusko added the ci:build_wheels Activate wheel building. label Jun 19, 2024

empty commit

03b5b8e

AmintorDusko marked this pull request as ready for review June 19, 2024 12:44

update changelog

55b50dd

AmintorDusko added 2 commits June 19, 2024 08:51

format catalyst files

4f1f279

comment out add subdir tests

48c0a2d

AmintorDusko requested a review from vincentmr June 20, 2024 13:41

vincentmr reviewed Jun 20, 2024

View reviewed changes

AmintorDusko and others added 9 commits June 20, 2024 16:20

update LKokkos catalyst build to make its include directories public

eb75e34

add cmake build file for LKokkos catalyst tests

ade1883

add runner_lightning_kokkos_catalyst

8109f4e

add constructibility and gates tests for LightningKokkosSimulator

b8a8c71

Auto update version from '0.37.0-dev44' to '0.37.0-dev46'

cec3ced

Merge branch 'master' into add/rtd_lightning_kokkos

263f865

expand coverage

70f3658

format

d6ddd85

trigger cis

cc8a855

AmintorDusko added the urgent Mark a pull request as high priority label Jun 21, 2024

AmintorDusko added 2 commits June 21, 2024 10:42

trigger cis

2b59c49

add measurements tests

6913d8a

AmintorDusko requested review from mlxd, dime10 and maliasadi June 25, 2024 11:13

AmintorDusko and others added 3 commits June 25, 2024 07:14

trigger CIs

2f9569c

Merge branch 'add/rtd_lightning_kokkos' of https://github.com/PennyLa…

068bccc

…neAI/pennylane-lightning into add/rtd_lightning_kokkos

Update code coverage

a5423c1

maliasadi reviewed Jun 25, 2024

View reviewed changes

Merge branch 'master' into add/rtd_lightning_kokkos

a56a266

mlxd reviewed Jun 26, 2024

View reviewed changes

AmintorDusko and others added 2 commits June 26, 2024 14:18

review suggestions

db9f17a

Auto update version from '0.37.0-dev49' to '0.37.0-dev50'

dcb468a

dime10 reviewed Jun 26, 2024

View reviewed changes

AmintorDusko and others added 5 commits June 27, 2024 12:55

more review suggestions

8481ea3

improve docstrings

022aa3c

Merge branch 'master' into add/rtd_lightning_kokkos

6d57152

format

44fe033

Merge branch 'add/rtd_lightning_kokkos' of https://github.com/PennyLa…

221efc2

…neAI/pennylane-lightning into add/rtd_lightning_kokkos

mlxd approved these changes Jun 27, 2024

View reviewed changes

AmintorDusko and others added 2 commits July 8, 2024 12:50

Merge branch 'master' into add/rtd_lightning_kokkos

f4db5e9

add changelog

7555e49

AmintorDusko removed the urgent Mark a pull request as high priority label Jul 8, 2024

vincentmr reviewed Jul 8, 2024

View reviewed changes

.github/CHANGELOG.md Outdated Show resolved Hide resolved

update changelog

ee1d8b5

vincentmr approved these changes Jul 8, 2024

View reviewed changes

AmintorDusko merged commit b9b6e1c into master Jul 8, 2024
88 of 93 checks passed

AmintorDusko deleted the add/rtd_lightning_kokkos branch July 8, 2024 19:10

paul0403 added a commit that referenced this pull request Jul 26, 2024

The lightning kokkos files in the Catalyst repo has changed since #770.…

32e3e7e

… We make a small update to track these changes. The Catalyst PR adds seeding to qjit: PennyLaneAI/catalyst#936

paul0403 mentioned this pull request Jul 26, 2024

Update kokkos files from catalyst repo to support MCM seeding #819

Merged

paul0403 mentioned this pull request Jul 30, 2024

Remove Lightning Kokkos plugin PennyLaneAI/catalyst#974

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Catalyst specific Lightning Kokkos class #770

Add Catalyst specific Lightning Kokkos class #770

AmintorDusko commented Jun 19, 2024 •

edited by maliasadi

Loading

github-actions bot commented Jun 19, 2024

codecov bot commented Jun 19, 2024 •

edited

Loading

AmintorDusko commented Jun 19, 2024 •

edited

Loading

AmintorDusko commented Jun 19, 2024

vincentmr left a comment

maliasadi left a comment

mlxd left a comment

dime10 left a comment

mlxd left a comment

vincentmr left a comment

vincentmr left a comment

Add Catalyst specific Lightning Kokkos class #770

Add Catalyst specific Lightning Kokkos class #770

Conversation

AmintorDusko commented Jun 19, 2024 • edited by maliasadi Loading

github-actions bot commented Jun 19, 2024

codecov bot commented Jun 19, 2024 • edited Loading

Codecov Report

AmintorDusko commented Jun 19, 2024 • edited Loading

AmintorDusko commented Jun 19, 2024

vincentmr left a comment

Choose a reason for hiding this comment

maliasadi left a comment

Choose a reason for hiding this comment

mlxd left a comment

Choose a reason for hiding this comment

dime10 left a comment

Choose a reason for hiding this comment

mlxd left a comment

Choose a reason for hiding this comment

vincentmr left a comment

Choose a reason for hiding this comment

vincentmr left a comment

Choose a reason for hiding this comment

AmintorDusko commented Jun 19, 2024 •

edited by maliasadi

Loading

codecov bot commented Jun 19, 2024 •

edited

Loading

AmintorDusko commented Jun 19, 2024 •

edited

Loading