Enable purely iteration-based training #5726
Conversation
Codecov Report
@@ Coverage Diff @@
## release/1.2-dev #5726 +/- ##
===============================================
- Coverage 88% 88% -0%
===============================================
Files 181 181
Lines 12929 12932 +3
===============================================
+ Hits 11426 11428 +2
- Misses 1503 1504 +1 |
ananthsub
requested review from
awaelchli,
Borda,
justusschock,
SeanNaren,
tchaton and
williamFalcon
as code owners
| # @pytest.mark.parametrize("min_epochs", [None, 2]) | ||
| # @pytest.mark.parametrize("max_epochs", [None, 3]) | ||
| # @pytest.mark.parametrize("min_steps", [None, 20]) | ||
| # @pytest.mark.parametrize("max_steps", [None, 100]) |
There was a problem hiding this comment.
Are all of these 16 configurations valid?
Did you try what happens if we choose max_epochs = 1 and max_steps = 5 but the epoch only has 3 batches?
The number that leads to less training steps should terminate training, right? This could be a separate test.
There was a problem hiding this comment.
if both are specified, i think should exit out whenever the first condition is met. so for the example you provided, if the epoch has only 3 batches, i agree we should return after the epoch completes and before moving onto the remaining 2 steps. the same goes for min_steps
i think users can already exercise this functionality now, so i'll look to see if there are already test cases for it. if not i'll add them here
|
one thing i need to check is what happens to the progress bar |
What does this PR do?
Fixes #3521
This preserves the existing default of
min_epochs=1if bothmin_stepsandmin_epochsaren't specified, and the default behavior ofmax_epochs=1000if bothmax_stepsandmax_epochsaren't specified.Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃