fix(torch): honor drop_last in all to_dataloader() modes#207
Merged
Conversation
Two defects: buffered modes ignore drop_last=False (unconditional n_keep truncation + ChunkPlanner divisibility requirement), and default mode crashes on drop_last=True (drop_last forwarded to DataLoader alongside batch_size=None). Design teaches ChunkPlanner about a trailing partial batch and stops forwarding drop_last in the default path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Summary
Dataset.to_dataloader()did not honordrop_lastconsistently across modes. This fixes both directions of the defect:drop_last=False. Now the trailing partial batch is kept.mode=None) crashed withdrop_last=Truebecausedrop_lastwas forwarded totd.DataLoaderalongsidebatch_size=None(PyTorch rejects that combination). TheBatchSampleris now the sole authority on dropping the partial batch.Changes
_chunked.py—ChunkPlannerno longer requires the index count to be a multiple ofbatch_size; it emits the trailing partial batch as a remainder entry inbatch_totalsand clamps the final chunk slice ton._torch.py—_resolve_buffered_inputsgates partial-batch truncation ondrop_last;get_dataloaderstops forwardingdrop_lasttotd.DataLoaderin default mode; a directly-passedBatchSampler'sbatch_sizeis adopted for buffered re-batching (with a warning when it conflicts with an explicitbatch_size)._buffered_loader.py—__len__floor → ceil so it matches what iteration yields.Test Plan
pytest tests/unit/test_chunk_planner.py tests/unit/test_torch.py tests/unit/test_buffered_loader.py tests/unit/test_double_buffered_loader.py→ 39 passed, 1 skipped (1kg-gated)mode × drop_lastmatrix, partial-batch instance count, default-modedrop_last=Trueregression, and a DDP-shaped custom-BatchSamplercaseceil(N/bs)batches withdrop_last=FalseandN//bswithdrop_last=Truein both default and buffered modes🤖 Generated with Claude Code