Fix stateful dataloader losing shuffle order on multi-process resume (#4195) - #4238
Open
massimiliano1991 wants to merge 1 commit into
Open
Conversation
…uggingface#4195) Under multi-process training, a stateful dataloader's shuffle order is seeded by DataLoaderShard/DataLoaderDispatcher.iteration (the epoch counter), propagated to the sampler via set_epoch(). That counter was never part of the checkpoint, so a fresh process resumed with iteration=0 and replayed epoch 0's permutation at the restored cursor position instead of continuing the current epoch's order. Persist the epoch counter as `_iteration` in the dataloader state_dict and restore it in load_state_dict() before the base dataloader rebuilds its sampler iterator. With `use_seedable_sampler=True` this makes the shuffle order resume exactly. For the non-seedable multi-process path (a plain RandomSampler whose generator state cannot be recovered across a process restart), emit a warning pointing at use_seedable_sampler. The adapter state_dict now carries one extra key beyond torchdata's StatefulDataLoader; the equivalence tests drop it before comparing the shared state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4 tasks
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.
What this fixes
Closes #4195 — under multi-process training, a shuffled stateful dataloader resumes on epoch 0's permutation at the restored cursor position instead of continuing the current epoch's order.
Root cause
In the sharded stateful path, the shuffle order for a given
__iter__is seeded byDataLoaderShard/DataLoaderDispatcher.iteration(the epoch counter), whichset_epoch()propagates down to the sampler (SeedableRandomSampler.epoch, or the sampler generator). That counter is never part of the checkpoint:DataLoaderStateMixin.state_dict()returns only the underlying torchdataStatefulDataLoaderstate, andload_state_dict()forwards it verbatim.So on resume a fresh process starts at
iteration == 0,__iter__callsset_epoch(0), and torchdata's fast-forward (itertools.islice(iter(index_sampler), sampler_iter_yielded, None)) re-iterates the sampler from epoch 0 — restoring the batch position but the epoch‑0 order. (Single-process is unaffected: there the sampler iterator state torchdata already serializes carries the permutation.)The fix
_iterationin the dataloaderstate_dict, and restore it inload_state_dict()(viaset_epoch) before the base dataloader rebuilds its sampler iterator. Withuse_seedable_sampler=Truethe shuffle order then resumes exactly.RandomSamplermulti-process path has no recoverable per-epoch seed across a process restart (its generator seed is drawn fresh each launch), so exact shuffle-resume isn't achievable there — emit a one-time warning pointing atuse_seedable_sampler=True. This is the issue's option 2.The adapter
state_dictnow carries one extra key beyond torchdata'sStatefulDataLoader; loading a state without_iteration(old checkpoints, or a torchdata dict) is a no-op, and the two*_equivalent_to_torchdata_stateful_dataloadertests drop that key before comparing the shared state.Verification
Against
main@f13f7c13, the issue's reproducer (torchrun --nproc_per_node=2,use_seedable_sampler=True):Verified for both
DataLoaderShardandDataLoaderDispatcher(dispatch_batches=True), on 2 processes. ExistingStatefulDataLoaderTestersuite passes (12 passed) with the equivalence tests updated.I did not add an automated regression test: the bug only manifests across a genuine process restart (a fresh loader at
iteration=0), which the current in-process dataloader test harness doesn't simulate — the existing_test_stateful_dataloader_resumecheckpoints within epoch 0 and so never exercises it. Happy to add one in whatever form you prefer (e.g. a two-launch save/resume intest_distributed_data_loop.py).Disclosure: I'm an AI agent working under the maintainer's GitHub account. The diagnosis and fix were verified at the code level against today's
mainas described above; please review on the merits and tell me if anything's off — I'll follow up.