-
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
Sequential CombinedLoader
to flatten the eval and predict loops
#16726
Conversation
CombinedLoader
to flatten the evaluation and prediction loops
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.
That's a monster to review but seems pretty good. Only one question I might have missed due to the massive amount of changes I just tried to follow and a few mypy nits :)
A strong LGTM from me!
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.
LGTM. This looks very promising.
CombinedLoader
to flatten the evaluation and prediction loopsCombinedLoader
to flatten the eval and predict loops
What does this PR do?
_EvaluationEpochLoop
_PredictionEpochLoop
_DataLoaderLoop
_EvaluationLoop
out of the dataloader directory_PredictionLoop
out of the dataloader directoryloop.setup_data()
method to the top-level loops. This method is called at the beginning ofloop.run()
. This replaces thetrainer.reset_*_dataloader
methods.trainer.reset_*_dataloader
methods and related trainer.py simplifications_DataLoaderSource
andCombinedLoader
trainer.*_dataloader
properties now return what the user returned in theirModule.*_dataloader()
hook. Nit: validation dataloaders are casted to list, see follow-up sectionpredict_step(dataloader_iter)
. this is not an important objective of the PR, but it's implemented for free.Fixes #10696
Fixes #11845
Follow-ups:
setup_data
implementations (Refactorloop.setup_data
with utility functions #16918)loop.setup_data
with utility functions #16918)num_*_batches
ownership to the loops (Movemax_batches
definition to the Loops #16820)Trainer.evaluation_dataloaders
should not be a forced list (RemoveTrainer(multiple_trainloader_mode)
in favor ofCombinedLoader(mode)
#16800)Trainer(multiple_trainloader_mode)
in favor ofCombinedLoader(mode)
#16800)_TrainingEpochLoop
#16801)CombinedLoader
to an utility file #16819)cc @justusschock @awaelchli @Borda @carmocca