remove reset_train_val_dataloaders from Trainer and move data reloading logic to loop

[ninginthecloud]

What does this PR do?

Fixes #9502

This PR aims to fix the bug mentioned in issue #9502.
During fit loop, train_dataloader is loaded twice when resuming from checkpoint. This underlying behavior does not meet user's expectation.
As discussed in an closed PR #9614, we will follow the second option to fix this issue:

• Option 2: remove self.reset_train_val_dataloaders(model) in Trainer._run_train() and set the reload logics in corresponding loops. Additionally,

1. self.num_training_batches = 0 in trainer._setup_on_init(). This value is initialized as self.num_training_batches = float('inf'),
2. fit_loop.done and fit_loop.skip has been updated.

@property
    def done(self) -> bool:
        """Evaluates when to leave the loop.

        Returns True if trainer.should_stop was set (e.g. by early stopping) or if the maximum number of steps or epochs
        is reached.
        """
        return stop_steps or should_stop or stop_epochs or self.trainer.num_training_batches == 0

@property
    def skip(self) -> bool:
        """Whether we should skip the training and immediately return from the call to :meth:`run`."""
        return self.done or self.trainer.limit_train_batches == 0

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

• Was this discussed/approved via a GitHub issue? (not for typos and docs)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure your PR does only one thing, instead of bundling different changes together?
• Did you make sure to update the documentation with your changes? (if necessary)
• Did you write any new necessary tests? (not for typos and docs)
• Did you verify new and existing tests pass locally with your changes?
• Did you list all the breaking changes introduced by this pull request?
• Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:

• Is this pull request ready for review? (if not, please submit in draft mode)
• Check that all items from Before submitting are resolved
• Make sure the title is self-explanatory and the description concisely explains the PR
• Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Make sure you had fun coding 🙃

[codecov]

Codecov Report

Merging #9671 (36f5fe4) into master (83ce1bf) will decrease coverage by 4%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #9671    +/-   ##
=======================================
- Coverage      93%     89%    -4%     
=======================================
  Files         179     180     +1     
  Lines       15810   15869    +59     
=======================================
- Hits        14635   14071   -564     
- Misses       1175    1798   +623     

[carmocca]

Marking this as a draft as a merge seems to have gone wrong, mark it as ready when it's ready again!

[ninginthecloud]

Marking this as a draft as a merge seems to have gone wrong, mark it as ready when it's ready again!

I've got rebase and made this PR in a clean stage. Looking for another round of review, thank you~ 😃 cc: @carmocca, @tchaton

[ninginthecloud]

Hi, @carmocca The value of self.trainer.num_training_batches is set when train_dataloader is loaded, otherwise, it's just default value. However, since we moved reset_train_val_dataloaders to fit_loop, it's impossible to get the valid evaluation of skip. Therefore, I replaced it with limit_train_batches==0 to skip loop based on the logic defined in data_loading.py

[tchaton]

LGTM !

[ninginthecloud]

a lot of test failures due to commit 720288e Let me update them