-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support multiple dataloaders with dataloader_iter
#18390
Support multiple dataloaders with dataloader_iter
#18390
Conversation
…ataloader_iter_multiple_dataloaders
⚡ Required checks status: All passing 🟢Groups summary🟢 pytorch_lightning: Tests workflowThese checks are required after the changes to 🟢 pytorch_lightning: Azure GPU
These checks are required after the changes to 🟢 pytorch_lightning: Benchmarks
These checks are required after the changes to 🟢 pytorch_lightning: Docs
These checks are required after the changes to 🟢 mypy
These checks are required after the changes to 🟢 installThese checks are required after the changes to Thank you for your contribution! 💜
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #18390 +/- ##
==========================================
- Coverage 85% 50% -35%
==========================================
Files 427 419 -8
Lines 33289 33186 -103
==========================================
- Hits 28291 16655 -11636
- Misses 4998 16531 +11533 |
if isinstance(data_fetcher, _DataLoaderIterDataFetcher): | ||
dataloader_iter = next(data_fetcher) | ||
# hook's batch_idx and dataloader_idx arguments correctness cannot be guaranteed in this setting | ||
batch = data_fetcher._batch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we store the fetched batch in memory before this PR as well? I think that previously the "batch" variable was actually the dataloader_iter. My understanding is that we need to store the batch so we can pass it to the callback hooks. An argument could probably be made that in case of dataloader_iter, the loop should probably not hold onto a batch (and perhaps pass None to the hooks).
What does this PR do?
Support multiple dataloaders with
dataloader_iter
This unblocks the NeMo team.
cc @justusschock @awaelchli @tchaton @Borda