Skip to content

Fix DispatchDataloader#588

Merged
sgugger merged 3 commits intomainfrom
fix_dispatch_dl
Aug 1, 2022
Merged

Fix DispatchDataloader#588
sgugger merged 3 commits intomainfrom
fix_dispatch_dl

Conversation

@sgugger
Copy link
Collaborator

@sgugger sgugger commented Aug 1, 2022

This PR fixes the dispatch dataloader which was a bit broken by the recent refactors. It's untested (since it requires a multiGPU setup) but will be in the new evaluation testing @muellerzr will add soon.

"""

def __init__(self, dataset, split_batches: bool = False, **kwargs):
def __init__(self, dataset, split_batches: bool = False, _drop_last: bool = False, **kwargs):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyTorch does not let us pass drop_last.

Comment on lines -503 to -506
if not self._stop_iteration:
yield batch
batch, batch_info, skip = next_batch, next_batch_info, next_skip
else:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this logic as we don't need to take the next batch to have an informed decision on whether we are the last batch or not. This was actually causing the dataloader to stop one batch too early.

@sgugger sgugger requested a review from muellerzr August 1, 2022 18:23
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Aug 1, 2022

The documentation is not available anymore as the PR was closed or merged.

Copy link
Contributor

@muellerzr muellerzr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for iterating and working with me on this 🤗

@sgugger sgugger merged commit 7a5a96b into main Aug 1, 2022
@sgugger sgugger deleted the fix_dispatch_dl branch August 1, 2022 19:55
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.

3 participants