Skip to content

Fix BatchSamplerShard tail sampling - #9907

Open
taking-lying-flat wants to merge 1 commit into
modelscope:mainfrom
taking-lying-flat:agent/fix-batch-sampler-shard
Open

Fix BatchSamplerShard tail sampling#9907
taking-lying-flat wants to merge 1 commit into
modelscope:mainfrom
taking-lying-flat:agent/fix-batch-sampler-shard

Conversation

@taking-lying-flat

@taking-lying-flat taking-lying-flat commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • preserve the full dataset size when constructing BatchSamplerShard
  • generate the global sample order before applying distributed sharding
  • truncate for drop_last=True and repeat-pad for drop_last=False
  • apply the same distributed semantics to shuffled, sequential, and length-grouped sampling
  • compute batch sampler length from the per-rank sample count

Root cause

BatchSamplerShard previously floored the dataset size by the DP world size in its constructor. The non-length-grouped paths then generated indices only for that shortened prefix, so tail samples were permanently excluded even when drop_last=False. For packed datasets, the omitted item could be a full packed bin.

Impact

All original samples now participate when drop_last=False, while every DP rank still receives the same number of samples. When drop_last=True, indices are truncated after the epoch-specific global order is generated.

@taking-lying-flat
taking-lying-flat marked this pull request as ready for review August 14, 2026 00:39
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.

1 participant