Skip to content

Capture stateful dataloader state lazily so prepare() does not draw the shuffle permutation - #4234

Open
jayzuccarelli wants to merge 2 commits into
huggingface:mainfrom
jayzuccarelli:fix-stateful-dl-eager-state-dict
Open

jayzuccarelli wants to merge 2 commits into
huggingface:mainfrom
jayzuccarelli:fix-stateful-dl-eager-state-dict

Conversation

@jayzuccarelli

Copy link
Copy Markdown

What does this PR do?

Fixes #4204

This is the lazy capture I proposed on the issue on Sep 2.

DataLoaderAdapter.__init__ calls base_dataloader.state_dict() right after building the StatefulDataLoader. With num_workers > 0 that builds the iterator and draws the sampler permutation at prepare() time, before synchronize_rng_states runs in __iter__, so each rank shards a different epoch-0 permutation.

Capture it lazily instead: dl_state_dict starts as None and _update_state_dict fills it once iteration starts. state_dict() falls back to the base dataloader if nothing was captured yet, so checkpointing before the first batch works as before.

Added a single-process test (shuffle order follows the seed set after init) and a 2-process num_workers > 0 case in test_distributed_data_loop.py. Both fail on main.

Before submitting

Who can review?

@SunMarc

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.

use_stateful_dataloader + num_workers>0 draws epoch-0 permutation during prepare() before cross-rank RNG sync — corrupts DDP data partition

1 participant