Skip to content

fix(processing): shuffle with a local RNG instead of seeding global generators - #945

Open
Yi-111-a wants to merge 6 commits into
Lightning-AI:mainfrom
Yi-111-a:fix-optimize-preserve-caller-random-state
Open

Yi-111-a wants to merge 6 commits into
Lightning-AI:mainfrom
Yi-111-a:fix-optimize-preserve-caller-random-state

Conversation

@Yi-111-a

@Yi-111-a Yi-111-a commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

optimize() and map() run DataProcessor.run in the caller's process, and run seeded the global random, NumPy and torch generators with random_seed (default 42) and never restored them. That threw away the seed the caller had set before the call, so everything random afterwards (model init, shuffling, sampling) was the same whatever seed the caller picked. It also changed depending on whether an optimize call had happened earlier in the run, for example only when a cache didn't exist yet.

litdata only needs randomness for one thing: shuffling the items assigned to each worker in the weighted / file-size mapping. That shuffle now uses a local np.random.RandomState(random_seed), and run no longer calls random.seed, np.random.seed or torch.manual_seed.

Fixes #944

Changes

  • DataProcessor.run: dropped the three global seeding calls. Builds a local np.random.RandomState(self.random_seed) and passes it to both _map_items_to_workers_weighted call sites (explicit weights and file-size reordering).
  • _map_items_to_workers_weighted: new optional rng argument. None keeps the old np.random behaviour for direct callers.
  • optimize() / map(): expose random_seed: int | None = 42 and forward it to DataProcessor. None gives a non-deterministic shuffle.
  • Docstrings for random_seed now say it only seeds the worker shuffle.

Behaviour notes

  • Chunk layout doesn't change. RandomState(42) produces the same sequence as np.random.seed(42), and nothing between the old seeding point and the shuffle draws from the global NumPy generator. So optimize / map assign and order items exactly as before.
  • Global generators are no longer seeded. Before, random_seed also seeded the parent process, so a recipe's prepare_structure and, under start_method="fork" (the default in notebooks), the forked workers' fn inherited the seeded state. Under the default spawn start method, fn never inherited it. Code that relied on litdata seeding its randomness should seed inside fn / the recipe.

Tests

In tests/processing/test_data_processor.py:

  • test_optimize_preserves_the_callers_random_state[0|1]: seeds the caller, runs optimize, and checks that the next draw from random, NumPy and torch is the one the caller would have got without the call. On the unpatched tree both seeds return random=0.639427, exactly as reported in the issue.
  • test_optimize_does_not_seed_the_global_generators: fails if random.seed, numpy.random.seed or torch.manual_seed is called during optimize. This also guards against torch queueing lazy CUDA / MPS / XPU seeds.
  • test_weighted_shuffle_uses_local_rng[weights|file_sizes]: covers both weighted branches. Checks that the caller's NumPy state is preserved, that a RandomState is forwarded, and that the shuffle is the same for the same random_seed whatever the caller seeded. It fails if rng= is dropped from either call site.

tests/processing: 150 passed, 4 skipped.

…mize

`optimize()` runs in the caller's process, but `DataProcessor.run` seeded
`random`, `numpy` and `torch` at the top of `run` and never restored them,
so the seed the caller chose was silently replaced. Everything random after an
`optimize` call was the same whatever seed was set, and differed depending on
whether an `optimize` had already run (only when a cache did not exist yet).

Save the states before seeding and restore them in a `finally`, so the fixed
seed only covers the processing work itself. The work moves to `_run` so the
restore covers every exit path, including an exception.

Fixes Lightning-AI#944
@codecov-commenter

codecov-commenter commented Oct 9, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82%. Comparing base (c6f075c) to head (2953581).
⚠️ Report is 1 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #945   +/-   ##
=====================================
  Coverage     82%     82%           
=====================================
  Files         72      72           
  Lines      16445   16442    -3     
=====================================
- Hits       13505   13503    -2     
+ Misses      2940    2939    -1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@deependujha

deependujha commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks for the PR and the clear write-up!

One gap: torch.manual_seed also affects GPU generators (CUDA, MPS, and XPU). If CUDA hasn't been initialized yet, torch applies the seed on first use. Since users typically call optimize before model.cuda(), the seed gets overwritten and isn't restored. I also reproduced this with MPS.

I think there's a simpler fix than restoring every generator: avoid changing global random state altogether.

  • Expose the seed. Add random_seed: int | None = 42 to optimize() and map() and pass it to DataProcessor. A single seed is sufficient since NumPy is the only RNG needed internally. It seems to have been missed.

  • Use a local RNG. The only randomness needed inside litdata is the worker shuffle in _map_items_to_workers_weighted, which uses np.random.permutation. Create rng = np.random.RandomState(self.random_seed) in DataProcessor and use rng.permutation(...) instead. The global seed calls and save/restore logic can then be removed.

  • Document the behavior change. With forked workers or num_workers=0, fn could inherit the seeded global state and was effectively getting seed 42. After this change, fn won't get that seed automatically. That's reasonable, callers should seed their own randomness if needed.

…the global generators

Expose random_seed on optimize() and map() and draw the per-worker shuffle from
np.random.RandomState(random_seed), so litdata never touches the caller's global
random / NumPy / torch (CPU or GPU) state. fn no longer inherits seed 42; seed it
yourself if needed.
@Yi-111-a

Copy link
Copy Markdown
Contributor Author

Thanks, that's a cleaner fix. Done in 3a976bf:

  • Exposed the seed. optimize() and map() now take random_seed: int | None = 42 and pass it to DataProcessor.
  • Local RNG. DataProcessor builds np.random.RandomState(self.random_seed) and hands it to _map_items_to_workers_weighted, which uses rng.permutation(...) (it falls back to np.random when no rng is passed, so direct callers keep working). The global random / np.random / torch.manual_seed calls and the save/restore block are gone, along with the now-unused random and torch imports in data_processor.py. With the same seed the shuffle is unchanged, since RandomState(42).permutation gives the same draws as np.random.seed(42) followed by np.random.permutation.
  • Documented the behavior change. The random_seed docstrings on optimize, map and DataProcessor say the global generators aren't seeded any more, so fn has to seed its own randomness if it depends on it. Previously, with forked workers or num_workers=0, it effectively inherited seed 42.

I also added test_optimize_does_not_seed_the_global_generators. It makes random.seed, numpy.random.seed and torch.manual_seed raise, which covers the CUDA/MPS/XPU case you reproduced without needing a GPU. The earlier state-preservation test is still there. I only had ruff available locally (no torch install), so I'm relying on CI for the test run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Weighted-shuffle RNG isolation lacks regression coverage, and the change leaves prepare_structure randomness uncontrolled by random_seed without documenting the altered determinism contract.

2 open issues
What changed in this PR

This PR addresses #944 by removing processing-time global RNG seeding and using a local NumPy generator for worker-item shuffling.

Changes:

  • Replace global seeding with a local shuffle generator.
  • Expose and document random_seed on map() and optimize().
  • Add regression tests for caller RNG preservation and avoidance of global seeders.
File Description
tests/​processing/​test_data_processor.py Adds caller RNG-state and global-seeder regression tests.
src/​litdata/​processing/​functions.py Exposes and forwards the shuffle-only random_seed option.
src/​litdata/​processing/​data_processor.py Removes global seeding and passes a local generator to weighted shuffling.

Review effort: Balanced

- fold the leftover _run helper back into run
- avoid reassigning the typed rng argument in _map_items_to_workers_weighted
- document that recipes must seed their own randomness
- add weighted / file-size shuffle coverage for local RNG forwarding
- silence S311 on the caller-state test
@deependujha deependujha changed the title fix(processing): leave the caller's global RNG state alone after optimize fix(processing): don't override the caller's random seed in optimize and map Oct 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are reported for the change to isolate worker-shuffle randomness from the caller’s global random state.

1 open issue
1 resolved since last review

Review effort: Balanced

@deependujha deependujha changed the title fix(processing): don't override the caller's random seed in optimize and map fix(processing): shuffle with a local RNG instead of seeding global generators Oct 28, 2026

@deependujha deependujha left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks :)

Comment thread tests/processing/test_data_processor.py Resolved
Comment thread tests/processing/test_data_processor.py Resolved
Comment thread tests/processing/test_data_processor.py Resolved
Co-authored-by: Deependu <deependujha21@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues were identified for the change isolating shuffle randomness from caller-managed random state.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DataProcessor.run reseeds the caller's global random, NumPy and torch generators

4 participants