Skip to content

[Trainer] Deeper length checks for IterableDatasetShard#15539

Merged
anton-l merged 3 commits intohuggingface:masterfrom
anton-l:trainer-len-checks
Feb 7, 2022
Merged

[Trainer] Deeper length checks for IterableDatasetShard#15539
anton-l merged 3 commits intohuggingface:masterfrom
anton-l:trainer-len-checks

Conversation

@anton-l
Copy link
Copy Markdown
Member

@anton-l anton-l commented Feb 7, 2022

What does this PR do?

As discussed in #15309 (comment) , in cases where IterableDatasetShard wraps a non-sized dataset, the check for isinstance(dataset, Sized) passes, while len(dataset) fails.
This PR adds a universal has_length(dataset) that tries to get the dataset's length explicitly without relying on type checking.

@HuggingFaceDocBuilder
Copy link
Copy Markdown

HuggingFaceDocBuilder commented Feb 7, 2022

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

Copy link
Copy Markdown
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Thanks a lot for creating a separate PR for this, it's way easier to review (and make sure it doesn't break anything) this way!

Comment thread src/transformers/trainer_utils.py Outdated
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
@anton-l anton-l merged commit 75b13f8 into huggingface:master Feb 7, 2022
@anton-l anton-l deleted the trainer-len-checks branch February 7, 2022 15:36
stevhliu pushed a commit to stevhliu/transformers that referenced this pull request Feb 18, 2022
…15539)

* Unused import

* Make `has_length()` torch-independent to use in callbacks

* Update src/transformers/trainer_utils.py

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
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