Repository navigation
Make stim sampling parallel by default - #108
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates ppvm-stim multi-shot sampling so that (when compiled with the optional rayon feature) it automatically uses parallel execution for large shot batches, and updates the Rust/Python user-facing docs + tests to reflect the new “shot-indexed factory” API and deterministic seeding strategy.
Changes:
- Refactors Stim sampling to support
sample_serial,sample_parallel(rayon-gated), and a defaultsamplethat chooses serial vs parallel based on batch size and thread count. - Updates Rust examples/docs/tests to use a factory closure
Fn(usize) -> GeneralizedTableau(shot index provided for deterministic per-shot seeds). - Enables rayon-backed sampling in the Python native extension and adds Python/Rust tests for reproducibility and serial/parallel equivalence.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/ppvm-usage/SKILL.md | Updates user documentation for the new shot-indexed factory and parallel sampling behavior. |
| skills/ppvm-usage/examples/rust/src/bin/stim_sample.rs | Updates example to use the new factory closure signature and explains deterministic seeding. |
| ppvm-python/test/generalized_tableau/test_stim.py | Adds a reproducibility test intended to cover the parallel sampling path. |
| ppvm-python/src/ppvm/generalized_tableau.py | Documents parallel sampling semantics and deterministic seed + i behavior. |
| crates/ppvm-stim/tests/executor.rs | Updates sampling tests for the new factory signature and adds serial-vs-parallel equivalence test (rayon). |
| crates/ppvm-stim/src/lib.rs | Re-exports sample_serial and (feature-gated) sample_parallel; updates doc example. |
| crates/ppvm-stim/src/executor.rs | Implements sample_serial, sample auto-dispatch, and sample_parallel (rayon). |
| crates/ppvm-stim/Cargo.toml | Adds optional rayon feature/dependency. |
| crates/ppvm-python-native/src/interface_tableau.rs | Routes Python sampling through ppvm_stim::sample with per-shot deterministic seeding and GIL release. |
| crates/ppvm-python-native/Cargo.toml | Enables ppvm-stim’s rayon feature for the Python native extension. |
| Cargo.lock | Records rayon as a dependency of ppvm-stim. |
| .gitignore | Ignores /debug. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ute_validated; add validated sample variants - Rename prepare.rs -> validate.rs and function prepare() -> validate() - Rename execute_prepared() -> execute_validated() throughout - Add sample_serial_validated / sample_parallel_validated / sample_validated that skip re-validation for callers that already hold a validated program - Python binding sample() now uses sample_validated() via the pre-validated StimProgram, avoiding redundant re-validation on every sampling call - Update test file prepare.rs -> validate.rs - Update all doc comments and unreachable! messages
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Roger-luo
reviewed
Jun 11, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 16 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
crates/ppvm-stim/src/validate.rs:21
validate()currently only checks probability ranges for measurement instructions andMPAD, but it does not validate probabilities for noise instructions (e.g.DEPOLARIZE1(p),X_ERROR(p), etc.) nor forI_ERROR[loss]/[correlated_loss]. This means invalid probabilities likeDEPOLARIZE1(2.0)can pass validation and then be executed, despiteExecError::InvalidProbabilityexisting for this purpose.
…ngs; update AGENTS.md and dev guide with docstring convention
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Merging main (#101 classical mixtures + sampling) added a `T::Coeff: PartialOrd` bound to `execute_validated`, but the `sample_serial_validated`, `sample`, `sample_validated`, and `sample_parallel*` wrappers only carried `PartialOrd<f64>`, so the crate stopped compiling. Add the missing `+ PartialOrd` to all seven affected functions. Also resolve outstanding review feedback: - Disambiguate the `[validate]` intra-doc links (now ambiguous between the `validate` module and `validate` fn after the prepare->validate rename) via the `fn@` disambiguator, silencing the rustdoc warning. - Rename `test_sample_stim_seeded_is_reproducible_across_the_parallel_path` -> `..._for_large_batches` and reword the comment: it asserts reproducibility whether rayon runs the batch serially or in parallel (the cutoff depends on pool size, which the test does not control). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR updates
ppvm-stimmulti-shot sampling so that (when compiled with the optionalrayonfeature) it automatically uses parallel execution for large shot batches, and updates the Rust/Python user-facing docs + tests to reflect the new "shot-indexed factory" API and deterministic seeding strategy.Changes
sample_serial,sample_parallel(rayon-gated), and a defaultsamplethat chooses serial vs parallel based on batch size and thread count.Fn(usize) -> GeneralizedTableau(shot index provided for deterministic per-shot seeds).sampleinto two#[cfg]-gated definitions so non-rayon builds do not requireSyncon the factory closure.prepare→validateandexecute_prepared→execute_validatedthroughout for naming clarity.sample_serial_validated,sample_parallel_validated, andsample_validatedvariants that accept pre-validated&[ExtendedInstruction]+measurement_countand skip re-validation. The Pythonsample()binding usessample_validated()so validation runs only once atStimProgram.parse()time instead of on every sampling call.mod 2⁶⁴) arithmetic.