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

paul0403 · 2024-07-26T13:49:51Z

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:

Benefits: unblocks kokkos with catalyst

Possible Drawbacks:

Related GitHub Issues:

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

github-actions · 2024-07-26T13:50:06Z

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-07-26T13:52:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.57%. Comparing base (d1b4007) to head (5688885).
Report is 88 commits behind head on master.

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

HEAD has 34 uploads less than BASE

Flag BASE (d1b4007) HEAD (5688885)

40 6

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #819       +/-   ##
===========================================
- Coverage   98.58%   87.57%   -11.01%     
===========================================
  Files         114       73       -41     
  Lines       18064    11368     -6696     
===========================================
- Hits        17808     9956     -7852     
- Misses        256     1412     +1156

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

vincentmr

So this seed is only used for the measure RNG. Is that intended? Otherwise looks good to me, but please update the change log.

maliasadi

Hmm... SetDevicePRNG is a virtual function with default impl. The first method of this kind in the QuantumDevice API.

/home/runner/work/pennylane-lightning/pennylane-lightning/Build/pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/include/QuantumDevice.hpp:125:46: error: unused parameter ‘gen’ [-Werror=unused-parameter]
  125 |     virtual void SetDevicePRNG(std::mt19937 *gen){};

You need to use [[maybe_unused]] for gen to get all actions PASS.

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

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

paul0403 · 2024-07-26T14:26:10Z

So this seed is only used for the measure RNG. Is that intended? Otherwise looks good to me, but please update the change log.

This is intended (sampling could not be seeded from the catalyst repo as it is done through Pennylane::LightningKokkos/Qubit::Measures::Measurements)

https://github.com/PennyLaneAI/catalyst/blob/a49e0bc9f828a28aa8cae7d07c64d09d84713ef4/runtime/lib/backend/lightning/lightning_dynamic/LightningSimulator.cpp#L304

paul0403 · 2024-07-26T14:30:16Z

Hmm... SetDevicePRNG is a virtual function with default impl. The first method of this kind in the QuantumDevice API.
/home/runner/work/pennylane-lightning/pennylane-lightning/Build/pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/include/QuantumDevice.hpp:125:46: error: unused parameter ‘gen’ [-Werror=unused-parameter]
  125 |     virtual void SetDevicePRNG(std::mt19937 *gen){};
You need to use [[maybe_unused]] for gen to get all actions PASS.

This should be updated in the catalyst repo I assume? Not that familiar with how the two repos see each other but seems like QuantumDevice.hpp is tracked from catalyst repo

… to uniform C-style initialization

…pennylane-lightning into update_catalyst_kokkos_seed

paul0403 · 2024-07-26T14:50:47Z

Hmm... SetDevicePRNG is a virtual function with default impl. The first method of this kind in the QuantumDevice API.

This is because not all devices can be seeded, so in the device base class we need to supply a default implementation of doing nothing, and let the seedable devices override them.

…evice.hpp/SetDevicePRNG(std::mt19937 *gen)` as `[[maybe_unused]]`, as some devices (e.g. openqasm and oqc) do not support seeding has this argument has no effect in the default implementation of this setter. At a more superficial level, this is to make the tests in PennyLaneAI/pennylane-lightning#819 pass.

.github/CHANGELOG.md

…antumDevice.hpp/SetDevicePRNG()` (#963) **Context:** Marking the `std::mt19937 *gen` argument in `runtime/include/QuantumDevice.hpp/SetDevicePRNG(std::mt19937 *gen)` as `[[maybe_unused]]`, as some devices (e.g. openqasm and oqc) do not support seeding and this argument has no effect in the default implementation of this setter. At a more superficial level, this is to make the tests in PennyLaneAI/pennylane-lightning#819 pass.

multiphaseCFD

Thanks @paul0403 ! LGTM.

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

vincentmr

LGTM, thanks @paul0403 .

maliasadi

🙌

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 requested review from dime10 and a team July 26, 2024 13:49

Auto update version from '0.38.0-dev17' to '0.38.0-dev18'

3777e7f

vincentmr suggested changes Jul 26, 2024

View reviewed changes

maliasadi requested changes Jul 26, 2024

View reviewed changes

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/LightningKokkosSimulator.hpp Outdated Show resolved Hide resolved

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/LightningKokkosSimulator.cpp Show resolved Hide resolved

paul0403 added 3 commits July 26, 2024 10:32

std::mt19937 *gen{nullptr} instead of std::mt19937 *gen = nullptr…

5dbb908

… to uniform C-style initialization

Merge branch 'update_catalyst_kokkos_seed' of github.com:PennyLaneAI/…

066f09d

…pennylane-lightning into update_catalyst_kokkos_seed

changelog

fd2df38

paul0403 mentioned this pull request Jul 26, 2024

Marking the std::mt19937 *gen argument as [[maybe_unused]]in `runtime/include/QuantumD… PennyLaneAI/catalyst#963

Merged

maliasadi reviewed Jul 26, 2024

View reviewed changes

.github/CHANGELOG.md Outdated Show resolved Hide resolved

maliasadi reviewed Jul 26, 2024

View reviewed changes

.github/CHANGELOG.md Outdated Show resolved Hide resolved

changelog typo and reordering

8d91735

multiphaseCFD reviewed Jul 26, 2024

View reviewed changes

pennylane_lightning/core/src/simulators/lightning_kokkos/catalyst/LightningKokkosSimulator.hpp Show resolved Hide resolved

vincentmr approved these changes Jul 26, 2024

View reviewed changes

paul0403 changed the title ~~Update kokkos files from catalyst repo~~ Update kokkos files from catalyst repo to support MCM seeding Jul 26, 2024

maliasadi approved these changes Jul 26, 2024

View reviewed changes

#include <random> in the .hpp

5688885

paul0403 merged commit 10dc3a9 into master Jul 26, 2024
83 of 84 checks passed

paul0403 deleted the update_catalyst_kokkos_seed branch July 26, 2024 17:31

paul0403 mentioned this pull request Aug 3, 2024

Add operations for setting state and basis state and integrate this with skip_initial_state_prep PennyLaneAI/catalyst#955

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

paul0403 commented Jul 26, 2024 •

edited

Loading

github-actions bot commented Jul 26, 2024

codecov bot commented Jul 26, 2024 •

edited

Loading

vincentmr left a comment

maliasadi left a comment •

edited

Loading

paul0403 commented Jul 26, 2024

paul0403 commented Jul 26, 2024

paul0403 commented Jul 26, 2024

multiphaseCFD left a comment

vincentmr left a comment

maliasadi left a comment

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

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

Conversation

paul0403 commented Jul 26, 2024 • edited Loading

github-actions bot commented Jul 26, 2024

codecov bot commented Jul 26, 2024 • edited Loading

Codecov Report

vincentmr left a comment

Choose a reason for hiding this comment

maliasadi left a comment • edited Loading

Choose a reason for hiding this comment

paul0403 commented Jul 26, 2024

paul0403 commented Jul 26, 2024

paul0403 commented Jul 26, 2024

multiphaseCFD left a comment

Choose a reason for hiding this comment

vincentmr left a comment

Choose a reason for hiding this comment

maliasadi left a comment

Choose a reason for hiding this comment

paul0403 commented Jul 26, 2024 •

edited

Loading

codecov bot commented Jul 26, 2024 •

edited

Loading

maliasadi left a comment •

edited

Loading