TrainerState refactor [5/5]

[carmocca]

What does this PR do?

TrainerState is now a dataclass which contains the TrainerStatus, TrainerFn (previous TrainerState), and RunningStage.
See the changes to pytorch-lightning/pytorch_lightning/trainer/states.py for an overview. All other changes are just fixing the code to work with that.

cc: @ananthsub. The TrainerState dataclass could be the one used for progress tracking in the future.

I'm open to different class names if the proposed ones are confusing.

Fixes

(Reported on Slack)

When KeyboardInterrupt is handled in the trainer, we set self.state = TrainerState.INTERRUPTED, thus losing the reference to the original TrainerState ({fit,validate,test,predict}). This means when teardown gets called, we don't know which stage should be set.

Fixes #7286

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?
• [n/a] Did you make sure to update the documentation with your changes? (if necessary)
• [n/a] 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 update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

• 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

[pep8speaks]

Hello @carmocca! Thanks for updating this PR.

• In the file pytorch_lightning/callbacks/model_checkpoint.py:

Line 292:13: W503 line break before binary operator

Comment last updated at 2021-05-04 10:05:34 UTC

[codecov]

Codecov Report

Merging #7173 (c3f6354) into master (a6aa1a0) will increase coverage by 0%.
The diff coverage is 92%.

@@          Coverage Diff           @@
##           master   #7173   +/-   ##
======================================
  Coverage      91%     91%           
======================================
  Files         200     200           
  Lines       12916   12922    +6     
======================================
+ Hits        11764   11792   +28     
+ Misses       1152    1130   -22     

[ananthsub]

Great refactor! I think this will make #7315 easier by providing a central dataclass we can build on top of.

How'd you find all the references to trainer.state or trainer.running_stage? grep? Deleting the properties and seeing what broke?

[carmocca]

How'd you find all the references to trainer.state or trainer.running_stage? grep? Deleting the properties and seeing what broke?

Bit of both :)

[Borda]

just need to clarify the back compatibility, how about usest who were asking trainer.state?

[tchaton]

Really neat !

[carmocca]

just need to clarify the back compatibility, how about usest who were asking trainer.state?

If users were relying on it, it will break but:

• I don't see how that could be avoided
• TrainerState is not in the docs
• I don't think we have ever advertised this as something people can use
• The previous TrainerState is not in any stable release yet - this is why It's important to get this into 1.3