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 seeded execution to the Catalyst runtime #936

Merged
merged 42 commits into from
Jul 25, 2024

Conversation

paul0403
Copy link
Contributor

@paul0403 paul0403 commented Jul 16, 2024

Context:
The test test_dynamic_one_shot_several_mcms in frontend/test/pytest/test_mid_circuit_measurement.py was marked as skipped because it was flaky: #842

After some investigation, it was found out that the test involves a measurement, which gives random results. The qjit run of the test is random, but the reference default.qubit run is seeded, so sometimes the results fall outside the acceptable range of np.allclose.

To resolve this, we add a seeding mechanism for qjit.

Description of the Change:
Implemented a random seeding infrastructure for qjit.

The top-level qjit decorator can now take in a string argument seed="some_string". The default value is empty string, which means an unseeded run.

The string will be propagated to the runtime ExecutionContext, which then initializes a PRNG (the c++ std::mt19937 pseudo random number generator) in the context.
The seed and the PRNG is then sent to individual devices.
When performing measurements, the devices draw according to this PRNG.

A proper seed is found and set, to deterministically resolve the flaky test test_dynamic_one_shot_several_mcms.

Benefits:

  • test_dynamic_one_shot_several_mcms now reliably passes
  • User can seed a qjit run

Possible Drawbacks:

Related GitHub Issues: closes #839

[sc-66696]

@paul0403 paul0403 added reviewer:require-wheels Pull Requests will need wheel building job successful before being merged author:build-wheels Run the wheel building workflows on this Pull Request labels Jul 16, 2024
Copy link

codecov bot commented Jul 16, 2024

Codecov Report

Attention: Patch coverage is 98.11321% with 2 lines in your changes missing coverage. Please review.

Project coverage is 97.90%. Comparing base (897630c) to head (276761e).

Files Patch % Lines
runtime/include/QuantumDevice.hpp 0.00% 1 Missing ⚠️
runtime/lib/capi/ExecutionContext.hpp 75.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #936   +/-   ##
=======================================
  Coverage   97.90%   97.90%           
=======================================
  Files          72       73    +1     
  Lines       10341    10371   +30     
  Branches     1180     1185    +5     
=======================================
+ Hits        10124    10154   +30     
- Misses        171      172    +1     
+ Partials       46       45    -1     

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

… it on github, as on my local machine I cannot reproduce the flaky failure.
@paul0403 paul0403 force-pushed the flaky_test_dynamic_one_shot branch from 601a897 to da4dd08 Compare July 16, 2024 19:54
@paul0403
Copy link
Contributor Author

paul0403 commented Jul 17, 2024

Confirming that this is what we want (using "draw" as a name for the internal "random state" in lightning simulator runtime) when we set a seed:

  • Each execution of a qjit-compiled function should not return the same results, as the user would expect repeated executions of things like catalyst.measure to be random
  • However, when a seed is set, the evolution history of repeated executions of a qjit-compiled function should stay the same.

i.e.

# in file test.py

dev = qml.device("lightning.qubit", wires=1, shots=None)
@qjit(seed="qwerty")
@qml.qnode(dev)
def circuit():
    qml.Hadamard(0)
    m = measure(0)
    @cond(m)
    def cfun0():
        qml.Hadamard(0)
    cfun0()
    return qml.probs()

print(circuit(), circuit(), circuit(), circuit())

If we run python test.py, the 4 executions should give "random" results from each other; however, running python test.py again should produce the same 4 results:

$ python3 test.py
[0.5 0.5] [0.5 0.5] [1. 0.] [0.5 0.5]
draw is 0.73658 draw is 0.797645 draw is 0.267068 draw is 0.793337 
$ python3 test.py
[0.5 0.5] [0.5 0.5] [1. 0.] [0.5 0.5]
draw is 0.73658 draw is 0.797645 draw is 0.267068 draw is 0.793337

The above is exact (shots=None). If we use a high number of shots (e,g, shots=10000) the probabilities will have small fluctuations:

$ python3 flaky_mcm.py
[0.5013 0.4987] [1. 0.] [1. 0.] [1. 0.]
draw is 0.838864 draw is 0.289431 draw is 0.405051 draw is 0.0953809 

$ python3 flaky_mcm.py
[0.5021 0.4979] [1. 0.] [1. 0.] [1. 0.]
draw is 0.838864 draw is 0.289431 draw is 0.405051 draw is 0.0953809

$ python3 flaky_mcm.py
[0.5019 0.4981] [1. 0.] [1. 0.] [1. 0.]
draw is 0.838864 draw is 0.289431 draw is 0.405051 draw is 0.0953809

$ python3 flaky_mcm.py
[0.4899 0.5101] [1. 0.] [1. 0.] [1. 0.]
draw is 0.838864 draw is 0.289431 draw is 0.405051 draw is 0.0953809

@dime10 @mudit2812

@dime10
Copy link
Collaborator

dime10 commented Jul 17, 2024

@paul0403 No, I would say each execution of the qjit function should produce the same results, since it is seeded.

However, if you were to invoke the same QNode multiple times within a single execution of the qjit function, they should produce different results.

@paul0403
Copy link
Contributor Author

@dime10 Ah, so

dev = qml.device("lightning.qubit", wires=1, shots=None)

@qjit(seed="qwerty")
def workflow():
    @qml.qnode(dev)
    def circuit():
        qml.Hadamard(0)
        m = measure(0)
        @cond(m)
        def cfun0():
            qml.Hadamard(0)
        cfun0()
        return qml.probs()

    return circuit(), circuit(), circuit(), circuit()

print(workflow())
print("#######")
print(workflow())
(Array([0.4975, 0.5025], dtype=float64), Array([1., 0.], dtype=float64), Array([1., 0.], dtype=float64), Array([1., 0.], dtype=float64))
#######
(Array([0.4975, 0.5025], dtype=float64), Array([1., 0.], dtype=float64), Array([1., 0.], dtype=float64), Array([1., 0.], dtype=float64))

i.e. the history of both workflow (which is the QJIT) calls are the same, but within each QJIT call the qnode results are different?

@dime10
Copy link
Collaborator

dime10 commented Jul 17, 2024

@dime10 Ah, so the history of both workflow (which is the QJIT) calls are the same, but within each QJIT call the qnode results are different?

Exactly :)

@mudit2812
Copy link
Contributor

@dime10 @paul0403 When we talked yesterday the behaviour I had in mind was more consistent with the first implementation. This is really more of a product question though.

@paul0403
Copy link
Contributor Author

@dime10 @paul0403 When we talked yesterday the behaviour I had in mind was more consistent with the first implementation. This is really more of a product question though.

@josh146

@dime10
Copy link
Collaborator

dime10 commented Jul 17, 2024

@dime10 @paul0403 When we talked yesterday the behaviour I had in mind was more consistent with the first implementation. This is really more of a product question though.

That would be harder to realize because there is no persistent state in the runtime across qjit invocations. (and also unnecessary imo)

The top-level qjit decorator can now take in a string argument `seed="some_string"`. The default value is empty string, which means an unseeded run.

The string will be propagated to the runtime `EvaluationContext`, which then initializes a PRNG (the c++ `std::mt19937` pseudo random number generator) in the context.
The seed and the PRNG is then sent to individual devices.
When performing measurements, the devices draw according to this PRNG.

A proper seed is found and set, to deterministically resolve the flaky test `test_dynamic_one_shot_several_mcms`.
@paul0403 paul0403 requested review from dime10 and josh146 July 18, 2024 19:43
@paul0403
Copy link
Contributor Author

paul0403 commented Jul 18, 2024

TODO: add frontend tests for seeded qjit; changelog done

@paul0403
Copy link
Contributor Author

Question: right now only measurements are seeded. Should sampling (aka shots) be seeded as well?

runtime/lib/capi/RuntimeCAPI.cpp Outdated Show resolved Hide resolved
frontend/catalyst/utils/gen_mlir.py Outdated Show resolved Hide resolved
runtime/lib/backend/common/Utils.hpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@dime10 dime10 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 @paul0403 !

I think there a few simplifications we can make, and the other question is whether we want to make the other update in the lightning device to use the same randomness for sampling (have you looked at whether this can easily be done?).

frontend/catalyst/third_party/oqc/src/OQCDevice.hpp Outdated Show resolved Hide resolved
frontend/catalyst/third_party/oqc/src/OQCDevice.cpp Outdated Show resolved Hide resolved
runtime/lib/backend/common/Utils.hpp Outdated Show resolved Hide resolved
runtime/lib/capi/ExecutionContext.hpp Outdated Show resolved Hide resolved
runtime/lib/capi/RuntimeCAPI.cpp Outdated Show resolved Hide resolved
runtime/lib/capi/RuntimeCAPI.cpp Outdated Show resolved Hide resolved
mlir/lib/Quantum/Transforms/ConversionPatterns.cpp Outdated Show resolved Hide resolved
@paul0403
Copy link
Contributor Author

paul0403 commented Jul 19, 2024

@erick-xanadu @dime10 Thanks for the suggestions! Yes, the root of the complexity is C not allowing overloading.

If we are fine with changing the signature to __catalyst__rt__initilize(char *) everywhere (and modifying all the necessary tests to reflect this), then this is by far the cleanest approach imo. The complex blocks in the conversion pass also become unnecessary.

I have done this in the most recent commit (without going over the tests right now).

…__rt__init, so the normal uses without supplying any arguments are possible
Copy link
Collaborator

@dime10 dime10 left a comment

Choose a reason for hiding this comment

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

Thanks @paul0403 :)

runtime/include/RuntimeCAPI.h Outdated Show resolved Hide resolved
runtime/lib/capi/ExecutionContext.hpp Outdated Show resolved Hide resolved
runtime/lib/capi/RuntimeCAPI.cpp Outdated Show resolved Hide resolved
runtime/lib/capi/RuntimeCAPI.cpp Outdated Show resolved Hide resolved
runtime/tests/Test_LightningMeasures.cpp Outdated Show resolved Hide resolved
@paul0403 paul0403 merged commit a49e0bc into main Jul 25, 2024
44 checks passed
@paul0403 paul0403 deleted the flaky_test_dynamic_one_shot branch July 25, 2024 21:52
paul0403 added a commit to PennyLaneAI/pennylane-lightning that referenced this pull request Jul 26, 2024
… We make a small update to track these changes.

The Catalyst PR adds seeding to qjit: PennyLaneAI/catalyst#936
paul0403 added a commit to PennyLaneAI/pennylane-lightning that referenced this pull request Jul 26, 2024
### 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>
paul0403 added a commit that referenced this pull request Aug 13, 2024
…t_several_mcms as skipped.

Seeding for qjit was added in #936, but only measurements were seeded, and samples were not. Hence this test is still flaky.
Sample seeding needs to be done in Lightning. #999
paul0403 added a commit that referenced this pull request Aug 13, 2024
**Context:**
Seeding for qjit was added in #936 , but only measurements were seeded,
and samples were not. Hence this test is still flaky. Sample seeding
needs to be done in Lightning. #999

**Description of the Change:**
Marking the test
test_mid_circuit_measurement.py/test_dynamic_one_shot_several_mcms as
xfail.

---------

Co-authored-by: David Ittah <dime10@users.noreply.github.com>
paul0403 added a commit to PennyLaneAI/pennylane-lightning that referenced this pull request Oct 3, 2024
…make the generated samples deterministic (#927)

### 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:**
[A while ago](PennyLaneAI/catalyst#936) 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](https://github.com/PennyLaneAI/catalyst/blob/a580bada575793b780d5366aa77dff6157cd4f93/runtime/lib/backend/common/Utils.hpp#L274)
, but samples were controlled by lightning.

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](https://github.com/PennyLaneAI/catalyst/blob/934726fe750043886415953dbd89a4c4ddeb9a80/runtime/lib/capi/ExecutionContext.hpp#L268).
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](https://github.com/PennyLaneAI/catalyst/blob/a580bada575793b780d5366aa77dff6157cd4f93/runtime/lib/backend/lightning/lightning_dynamic/LightningSimulator.hpp#L54).

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]

---------

Co-authored-by: ringo-but-quantum <github-ringo-but-quantum@xanadu.ai>
dime10 pushed a commit that referenced this pull request Oct 3, 2024
**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]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author:build-wheels Run the wheel building workflows on this Pull Request reviewer:require-wheels Pull Requests will need wheel building job successful before being merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Further investigate the flaky test test_dynamic_one_shot_several_mcms
4 participants