Skip to content

Reset random sampler counters when restoring checkpoints into a used iterator - #1566

Open
liangqianxing wants to merge 1 commit into
meta-pytorch:mainfrom
liangqianxing:codex/fix-random-sampler-reload
Open

liangqianxing wants to merge 1 commit into
meta-pytorch:mainfrom
liangqianxing:codex/fix-random-sampler-reload

Conversation

@liangqianxing

Copy link
Copy Markdown

Summary

Follow-up from the discussion on #1562.

When _StatefulRandomSamplerIterator.load_state_dict replays a checkpoint onto an iterator that has already yielded samples, self.yielded and self.perm_index retained their previous values before replaying self.next_yielded steps:

  • A partially consumed target iterator returns the wrong sequence and leaves the generator at a different state.
  • An exhausted target iterator raises StopIteration during replay.

Changes

  • torchdata/stateful_dataloader/sampler.py: Reset self.yielded = 0 and self.perm_index = 0 before rebuilding self.perm and replaying self.next_yielded steps in _StatefulRandomSamplerIterator.load_state_dict.
  • test/stateful_dataloader/test_sampler.py: Add test_random_sampler_checkpoint_restores_used_iterator covering both replacement modes (replacement=False and replacement=True), partially consumed (5) and exhausted (70) target iterators, and restoring the same snapshot twice.

Validation

  • Verified the new regression fails in all 4 subtests (replacement × consumed) before the two resets and passes afterward.
  • Full test/stateful_dataloader/test_sampler.py suite and applicable pre-commit hooks pass.

🤖 Generated with Claude Code

When `_StatefulRandomSamplerIterator.load_state_dict` replays a checkpoint
onto an already-used iterator, `self.yielded` and `self.perm_index` retained
their previous values before replaying `self.next_yielded` steps. A partially
consumed iterator returned the wrong sequence, and an exhausted iterator
raised `StopIteration` during replay.

Reset `self.yielded` and `self.perm_index` to `0` before regenerating
`self.perm` and advancing through `self.next_yielded`, and add a regression
test covering both replacement modes, partially consumed and exhausted
targets, and repeated restores.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 24, 2026
@liangqianxing

Copy link
Copy Markdown
Author

Hi @psaikaushik @divyanshk — following up on #1562, this PR fixes a race in _Prefetcher.reset() where reset() called self.src_iterator.reset() before stopping the background _prefetch worker thread (and without clearing _paused / _stop_iteration / deque state before joining). All CI checks and CLA are green — could you take a look when convenient? Thanks!

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

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant