Allow `lightning.qubit/kokkos::generate_samples` to take in seeds to make the generated samples deterministic #927

paul0403 · 2024-09-30T19:13:43Z

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:
A while ago a new seed option to qjit was added. The seed was used to make measurement results deterministic, but samples were still probabilistic. This is because within a qjit context, measurements were controlled from the catalyst repo , but samples were not.

To resolve stochastically failing tests (i.e. flaky tests) in catalyst, we add seeding for samples in lightning.

Description of the Change:
When qjit(seed=...) receives a (unsigned 32 bit int) seed value from the user, the seed gets propagated through mlir and generates a std::mt19937 rng instance in the catalyst execution context. This rng instance eventually becomes a field of the Catalyst::Runtime::Simulator::LightningSimulator (and kokkos) class catalyst/runtime/lib/backend/lightning/lightning_dynamic/LightningSimulator.hpp.

To seed samples, catalyst uses this device rng instance on the state vector's generate_samples methods: PennyLaneAI/catalyst#1164.

In lightning, the generate_samples method now takes in a seeding number. The catalyst devices pass in a seed into the lightning generate_samples; this seed is created deterministically from the aforementioned already seeded catalyst context rng instance. This makes the generated samples deterministc.

Benefits:
Fewer (hopefully no) stochatically failing frontend tests in catalyst.

Possible Drawbacks:

Related GitHub Issues:
[sc-72878]

github-actions · 2024-09-30T19:13:59Z

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-09-30T19:16:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.36%. Comparing base (fdf09bc) to head (c4d67f4).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #927      +/-   ##
==========================================
+ Coverage   96.41%   97.36%   +0.95%     
==========================================
  Files         215      215              
  Lines       28172    28211      +39     
==========================================
+ Hits        27162    27468     +306     
+ Misses       1010      743     -267

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

pennylane_lightning/core/src/simulators/lightning_qubit/measurements/MeasurementsLQubit.hpp

multiphaseCFD

Thanks @paul0403 ! Just a few Q.

pennylane_lightning/core/src/measurements/MeasurementsBase.hpp

multiphaseCFD

Thanks @paul0403 .

pennylane_lightning/core/src/simulators/lightning_qubit/measurements/MeasurementsLQubit.hpp

pennylane_lightning/core/src/measurements/MeasurementsBase.hpp

split setRandomSeed and setRNG into different functions

pennylane_lightning/core/src/measurements/MeasurementsBase.hpp

dime10

Nice seeing this will be pretty straightforward 👍

pennylane_lightning/core/src/measurements/MeasurementsBase.hpp

pennylane_lightning/core/src/simulators/lightning_qubit/measurements/MeasurementsLQubit.hpp

mlxd

Nice work @paul0403
Just a few quick questions:

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

pennylane_lightning/core/src/simulators/lightning_kokkos/measurements/MeasurementsKokkos.hpp

pennylane_lightning/core/src/simulators/lightning_qubit/measurements/MeasurementsLQubit.hpp

`const` to shots arguments

multiphaseCFD

LGTM ! Thanks @paul0403 !

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

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

mlxd

Thanks @paul0403
Last review from me --- I'll be happy once the following changes go in.

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

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

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

…yst test

… as well

**Context:** There's still quite a few frontend tests stochastically failing because the `seed` option in `qjit` only controls the measurements, but not the samples. We add seeding to the samples. **Description of the Change:** When `qjit(seed=...)` receives a (unsigned 32 bit int) seed value from the user, the seed gets propagated through mlir and [eventually becomes a field of the `Catalyst::Runtime::Simulator::LightningSimulator` class, alongside the seeded `std::mt19937` rng instance ](https://github.com/PennyLaneAI/catalyst/blob/a580bada575793b780d5366aa77dff6157cd4f93/runtime/lib/backend/lightning/lightning_dynamic/LightningSimulator.hpp#L54). This was done in #936. In #936 , [the device's rng instance is used during measurements ](https://github.com/PennyLaneAI/catalyst/blob/a580bada575793b780d5366aa77dff6157cd4f93/runtime/lib/backend/lightning/lightning_dynamic/LightningSimulator.cpp#L451), but not during samples. This is because samples are performed from the `Pennylane::LightningQubit::Measures::Measurements` class through the `generate_samples` methods, which is controlled by the lightning repo. To seed samples, we use the device rng instance to generate a deterministic seed to pass it onto the state vector's `generate_samples` methods. This is the only change in catalyst. In lightning, the `generate_samples` method now can take in a seeding number. The catalyst devices pass in a seed into the lightning `generate_samples`; this seed is created deterministically from the aforementioned already seeded catalyst context rng instance. This makes the generated samples deterministc. The above is published on the lightning repo as the branch "seed_sample_lightning": PennyLaneAI/pennylane-lightning#927 PennyLaneAI/pennylane-lightning@6f3e0d5 **Benefits:** Fewer (hopefully no) stochatically failing frontend tests. **Related GitHub Issues:** #999 [sc-72878]

paul0403 added 2 commits September 30, 2024 13:52

try getting catalyst's rng for generating samples

290a497

set default to nullptr

133bc29

paul0403 requested a review from a team September 30, 2024 19:13

Auto update version from '0.39.0-dev33' to '0.39.0-dev34'

d0f0777

paul0403 mentioned this pull request Sep 30, 2024

Seed samples for lightning.qubit/kokkos PennyLaneAI/catalyst#1164

Merged

make the other generate_samples take in the rng as well

ea8290b

paul0403 commented Sep 30, 2024

View reviewed changes

pennylane_lightning/core/src/simulators/lightning_qubit/measurements/MeasurementsLQubit.hpp Outdated Show resolved Hide resolved

multiphaseCFD reviewed Sep 30, 2024

View reviewed changes

pennylane_lightning/core/src/measurements/MeasurementsBase.hpp Outdated Show resolved Hide resolved

pennylane_lightning/core/src/measurements/MeasurementsBase.hpp Outdated Show resolved Hide resolved

paul0403 added 4 commits September 30, 2024 16:05

undo kokkos changes for now

ce4c532

remove unnecessary this->

9e5eb33

passing pointers as const reference

6aa6d47

format

cfb16b4

multiphaseCFD reviewed Sep 30, 2024

View reviewed changes

paul0403 marked this pull request as draft September 30, 2024 20:57

paul0403 added 2 commits September 30, 2024 16:59

convert to std::optional to avoid (pointers + default value of nullptr);

3047599

split setRandomSeed and setRNG into different functions

give std::nullopt default to std::optional

0b943f1

paul0403 marked this pull request as ready for review September 30, 2024 21:28

paul0403 commented Sep 30, 2024

View reviewed changes

pennylane_lightning/core/src/measurements/MeasurementsBase.hpp Outdated Show resolved Hide resolved

paul0403 and others added 2 commits October 1, 2024 11:43

test

f496df8

Auto update version from '0.39.0-dev34' to '0.39.0-dev35'

e116f5c

paul0403 requested a review from a team October 1, 2024 15:46

paul0403 added 3 commits October 1, 2024 11:51

Merge remote-tracking branch 'origin/master' into seed_sample_lightning

3c52db6

apply clang tidy comments

5216cca

format

234ced7

dime10 reviewed Oct 1, 2024

View reviewed changes

pennylane_lightning/core/src/measurements/MeasurementsBase.hpp Outdated Show resolved Hide resolved

pennylane_lightning/core/src/simulators/lightning_qubit/measurements/MeasurementsLQubit.hpp Outdated Show resolved Hide resolved

paul0403 added 2 commits October 1, 2024 15:38

pass the seeding integer, instead of the rng instance; add kokkos

c4c5341

codefactor

2a2420e

paul0403 changed the title ~~Allow lightning to receive an already seeded std::mt19937 rng instance from catalyst~~ Allow lightning.qubit/kokkos::generate_samples to take in seeds to make the generated samples deterministic Oct 1, 2024

mlxd reviewed Oct 2, 2024

View reviewed changes

mlxd added ci:build_wheels Activate wheel building. ci:use-gpu-runner Enable usage of GPU runner for this Pull Request labels Oct 2, 2024

maliasadi added the urgent Mark a pull request as high priority label Oct 2, 2024

paul0403 added 2 commits October 2, 2024 11:23

changelog author

9e98e33

auto for kokkos random pool type when type is obvious;

e57ada4

`const` to shots arguments

multiphaseCFD approved these changes Oct 2, 2024

View reviewed changes

dime10 reviewed Oct 2, 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 and others added 7 commits October 2, 2024 12:09

catalyst kokkos seed sample test

f082ea1

format

5edfdfa

Auto update version from '0.39.0-dev35' to '0.39.0-dev36'

3fca3d4

Merge remote-tracking branch 'origin/master' into seed_sample_lightning

c2f8e1a

change helper to private

5101ff4

fix lightning.gpu test failure

a127f33

optional header

c70de5d

mlxd requested changes Oct 2, 2024

View reviewed changes

paul0403 and others added 8 commits October 3, 2024 10:06

Merge remote-tracking branch 'origin/master' into seed_sample_lightning

82a3ed0

factor out common furnitures for all loop iterations for kokkos catal…

3be52d6

…yst test

remove const for size_t

92b64f7

rewrite the measurement seeding kokkos test to reuse common furniture…

edfa8fd

… as well

Auto update version from '0.39.0-dev36' to '0.39.0-dev37'

5306829

small bug

ddc6732

Merge remote-tracking branch 'origin/master' into seed_sample_lightning

3362dd0

Auto update version from '0.39.0-dev37' to '0.39.0-dev38'

c4d67f4

paul0403 requested a review from mlxd October 3, 2024 17:39

mlxd approved these changes Oct 3, 2024

View reviewed changes

paul0403 merged commit 6f3e0d5 into master Oct 3, 2024
125 checks passed

paul0403 deleted the seed_sample_lightning branch October 3, 2024 17:48

paul0403 mentioned this pull request Oct 4, 2024

Integrate LGPU to Catalyst #928

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow `lightning.qubit/kokkos::generate_samples` to take in seeds to make the generated samples deterministic #927

Allow `lightning.qubit/kokkos::generate_samples` to take in seeds to make the generated samples deterministic #927

paul0403 commented Sep 30, 2024 •

edited

Loading

github-actions bot commented Sep 30, 2024

codecov bot commented Sep 30, 2024 •

edited

Loading

multiphaseCFD left a comment

multiphaseCFD left a comment

dime10 left a comment

mlxd left a comment

multiphaseCFD left a comment

mlxd left a comment

Allow lightning.qubit/kokkos::generate_samples to take in seeds to make the generated samples deterministic #927

Allow lightning.qubit/kokkos::generate_samples to take in seeds to make the generated samples deterministic #927

Conversation

paul0403 commented Sep 30, 2024 • edited Loading

Before submitting

github-actions bot commented Sep 30, 2024

codecov bot commented Sep 30, 2024 • edited Loading

Codecov Report

multiphaseCFD left a comment

Choose a reason for hiding this comment

multiphaseCFD 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

multiphaseCFD left a comment

Choose a reason for hiding this comment

mlxd left a comment

Choose a reason for hiding this comment

Allow `lightning.qubit/kokkos::generate_samples` to take in seeds to make the generated samples deterministic #927

Allow `lightning.qubit/kokkos::generate_samples` to take in seeds to make the generated samples deterministic #927

paul0403 commented Sep 30, 2024 •

edited

Loading

codecov bot commented Sep 30, 2024 •

edited

Loading