Handle KeyboardInterrupt during training

[moi90]

Before submitting

• Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure to update the docs?
• Did you write any new necessary tests?
• If you made a notable change (that affects users), did you update the CHANGELOG?

What does this PR do?

Fixes #2079 by introducing a on_interrupted callback where the cause of the interruption can be examined, e.g. by using sys.exc_info().

[pep8speaks]

Hello @moi90! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-06-14 15:23:13 UTC

[mergify]

This pull request is now in conflict... :(

[Borda]

LGTM 🤖

[codecov]

Codecov Report

Merging #2134 into master will increase coverage by 0%.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #2134   +/-   ##
======================================
  Coverage      87%     87%           
======================================
  Files          68      68           
  Lines        5180    5187    +7     
======================================
+ Hits         4508    4515    +7     
  Misses        672     672           

[awaelchli]

looks great, also the testing is nice.
I propose to change the name to simply on_interrupt for consistency, because for example we have on_test_end and not on_test_ended.

[moi90]

@awaelchli Yes, you are right. I will change it accordingly.

[Borda]

looks great, also the testing is nice.
I propose to change the name to simply on_interrupt for consistency, because for example we have on_test_end and not on_test_ended.

I think it is about calling something extra on the interrupt, but true that the interruption status is in Trainer...

[mergify]

This pull request is now in conflict... :(

[moi90]

@Borda What is your point? Should it be on_interrupt or on_interrupted?

[Borda]

@Borda What is your point? Should it be on_interrupt or on_interrupted?

I would choose on_interrupted
cc: @PyTorchLightning/core-contributors @williamFalcon

[awaelchli]

@Borda why so?

arguments for on_interrupt:

• consistent with rest of callback naming, i.e. interrupt is the noun that refers to the interrupt the same way batch_end refers to the end.
• easier to remember since it follows the name pattern of the other callbacks
• closer to English language (can form a sentence)
• shorter/simpler

arguments for on_interrupted:

• ?

[williamFalcon]

on_interrupt to be consistent is my vote. all callbacks are set in the present tense for PL.

[Borda]

I said I would choose on_interrupted, but do not have any strong preference...

[awaelchli]

looks great, also the test.
thanks!

[williamFalcon]

@Borda can you rename to on_keyboard_interrupt?

unless this catches any general interrupt?

[Borda]

Borda can you rename to on_keyboard_interrupt?
unless this catches any general interrupt?

@williamFalcon renamed
cc: @awaelchli @moi90 @justusschock

[SkafteNicki]

LGTM

[moi90]

Great, thanks for merging!