Vectorize interleave_datasets index generation (probabilities + first/all_exhausted)#8318
Vectorize interleave_datasets index generation (probabilities + first/all_exhausted)#8318saforem2 wants to merge 2 commits into
Conversation
…/all_exhausted) `_interleave_map_style_datasets` builds the output index list in a pure-Python for-loop (one iteration per output row) when `probabilities` is given. For large interleaves this dominates runtime -- e.g. interleaving NVIDIA OpenMathInstruct-2 (~14M rows) with `all_exhausted` produces ~93M rows and takes ~90 min, almost all of it in that loop (the RNG is already batched; it is Python interpreter overhead, not compute). The sibling `probabilities is None` `all_exhausted` branch is already vectorized with numpy (modulo/offset). This brings the probabilities-given `first_exhausted` and `all_exhausted` branches to parity: replay the same 1000-sized `rng.choice(..., p=probabilities)` draw blocks, find the stop position from each source's length-th occurrence (min for first_exhausted, max for all_exhausted), and map each source's k-th appearance to `(k % length) + offset` with numpy. Output is bit-identical for a fixed `seed` (same RNG consumption + same rolling-window mapping): the existing hardcoded tests `test_interleave_datasets_probabilities` and `..._probabilities_oversampling_strategy` pass unchanged, and 80 randomized (lengths, probabilities, seed) cases across both strategies match the previous implementation exactly. `all_exhausted_without_replacement` keeps the explicit loop (its skip-on-exhaustion semantics make the output length data-dependent). Benchmark (3-source mix, ~93M output rows): ~90 min -> ~5 s. Adds a randomized determinism/balance test for the probabilities-given paths.
There was a problem hiding this comment.
Pull request overview
This PR accelerates interleave_datasets for map-style Datasets when probabilities is provided by replacing the per-row Python loop in _interleave_map_style_datasets with a NumPy-based, blockwise RNG replay + vectorized index mapping for stopping_strategy="first_exhausted" and "all_exhausted".
Changes:
- Vectorizes index generation for probabilities-driven interleaving under
first_exhaustedandall_exhausted, keepingall_exhausted_without_replacementon the explicit loop. - Adds a regression test to guard determinism and basic proportionality properties under both stopping strategies across multiple seeds.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/datasets/arrow_dataset.py | Introduces vectorized RNG replay and index mapping for probabilities-based first_exhausted/all_exhausted interleaving. |
| tests/test_arrow_dataset.py | Adds a regression test ensuring deterministic outputs/fingerprints and basic probability ordering across strategies/seeds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for s in range(n_datasets): | ||
| occ = np.flatnonzero(draws == s) | ||
| if len(occ) >= lengths[s]: | ||
| exhaust_pos[s] = occ[lengths[s] - 1] |
There was a problem hiding this comment.
I benchmarked the argsort-based single-pass approach against the current per-source np.flatnonzero, and the per-source version is actually faster in both the few-dataset and many-dataset cases, so I've kept it.
At 93M draws (dummy lengths, index-mapping section only):
| case | per-source flatnonzero (current) |
argsort + bincount |
|---|---|---|
| 3 datasets | 1.47s | 5.22s |
| 50 datasets | 7.58s | 12.13s |
The O(n_datasets * n_draws) scaling is real in complexity terms, but the constant factors favor flatnonzero: it's a cheap fully-vectorized compare-and-pack, whereas the argsort does an O(n_draws log n_draws) sort of a 93M-element array that dominates even at 50 datasets. flatnonzero's boolean temporaries are n_draws bools each (transient, one source at a time), so peak memory stays modest.
flatnonzero would only lose for a very large number of datasets (roughly where n_datasets approaches log(n_draws)); interleave mixes are typically a handful of sources, so this stays on the fast side in practice. Happy to switch to the argsort form if you'd prefer the better asymptotic behavior for pathological n_datasets regardless -- but for the common case the current code is the faster of the two.
- Empty source (length 0): the previous vectorized code crashed on np.concatenate([]) (blocks never populated), and stock crashed with a cryptic `IndexError: Index N out of range`. Now raise a clear ValueError naming the empty dataset indices, for both first_exhausted and all_exhausted (an empty source is degenerate either way; silently dropping it would change results). Added a parametrized test. - Tightened the stop-position comment (removed the in-line "minus... no:" thought process) to a clear final statement per strategy. Re the suggestion to replace the per-source np.flatnonzero grouping with an argsort-based single pass: benchmarked both at 93M draws -- flatnonzero is actually faster (3 datasets: 1.5s vs 5.2s; 50 datasets: 7.6s vs 12.1s), since the O(n log n) sort dominates while the per-source vectorized compare stays cheap well past 50 datasets. Keeping flatnonzero; will note this on the thread. Equivalence unchanged: 80/80 randomized cases + the existing hardcoded tests still match the previous implementation bit-for-bit.
|
Hi @lhoestq — gentle ping on this one when you have a moment. It vectorizes the
Looks like CI is awaiting maintainer approval to run (external contributor). Happy to rebase or adjust anything — thanks! |
What
_interleave_map_style_datasets(used byinterleave_datasetsfor map-styleDatasets) builds the output index list in a pure-Pythonfor-loop, one iteration per output row, whenprobabilitiesis given:For large interleaves this loop dominates runtime. Interleaving e.g.
nvidia/OpenMathInstruct-2(~14M rows) with two smaller datasets understopping_strategy="all_exhausted"produces ~93M output rows and the index build alone takes ~90 min — almost entirely Python interpreter overhead, since the RNG is already drawn in batches (rng.choice(..., size=1000)); it is not compute-bound.Notably, the sibling
probabilities is Noneall_exhaustedbranch in the same function is already vectorized with numpy (np.mod/offset). This PR brings the probabilities-givenfirst_exhaustedandall_exhaustedbranches to parity.How
Reproduce the exact same algorithm with numpy:
np.random.default_rng(seed)drawn in the same 1000-sizedrng.choice(n, p=probabilities)blocks — until the stop condition can be evaluated.length-th occurrence:minover sources forfirst_exhausted(stop when any source is exhausted),maxforall_exhausted(stop when every source is).(k % length) + offset(the same rolling-window-on-exhaustion behavior), vectorized per source.all_exhausted_without_replacementkeeps the explicit loop — its skip-on-exhaustion semantics make the output length data-dependent, so it isn't a simple function of the draw sequence.Bit-identical output
Because the RNG is consumed in exactly the same way and the index mapping is the same, output is bit-identical for a fixed
seed:test_interleave_datasets_probabilities([10, 11, 20, 12, 0, 21, 13]) andtest_interleave_datasets_probabilities_oversampling_strategy(16-element sequence) pass unchanged.(num_datasets, lengths, probabilities, seed)cases across bothfirst_exhaustedandall_exhaustedmatch the previous implementation exactly.test_interleave_datasets_probabilities_is_deterministic_and_balanced(parametrized over strategy + seed) as a regression guard.Benchmark
3-source mix, ~93M output rows (
all_exhausted, probabilities given): ~90 min → ~5 s.Notes
_fingerprint.first_exhausted/all_exhaustedonly;all_exhausted_without_replacementand theprobabilities is Nonepaths are untouched.