model checkpint on rank_zero_only & global rank state

[Borda]

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 #1366. As suggested in the issue
MOreover it it defines global proc rank so no instance needs to carry it...

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

[codecov]

Codecov Report

Merging #1408 into master will increase coverage by 0%.
The diff coverage is 95%.

@@          Coverage Diff           @@
##           master   #1408   +/-   ##
======================================
  Coverage      89%     89%           
======================================
  Files          69      70    +1     
  Lines        4148    4138   -10     
======================================
- Hits         3677    3670    -7     
+ Misses        471     468    -3     

[awaelchli]

using the rank_zero decorator is maybe not a bad idea. it could be moved out of logger package to a more general package. and the ModelCheckpoint could get a rank property.

[pep8speaks]

Hello @Borda! Thanks for updating this PR.

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

Comment last updated at 2020-04-24 20:53:37 UTC

[justusschock]

LGTM

[williamFalcon]

I think there are a few changes here that can break functionality.
I'd prefer @neggert to look at this closer since he spent a long time on this a while back

[mergify]

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

[Borda]

changelog need to be rebased on new release #1419

[mergify]

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

[mergify]

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

[awaelchli]

@Borda I saw the problem with my proposal and fixed it in my branch here:
https://github.com/awaelchli/pytorch-lightning/tree/delete-checkpoint-proposal
It is forked from your branch.
The decorator you had before was better than mine. It can be applied to both methods and functions.
I added the rank as the function attribute. What do you think?

[Borda]

@williamFalcon updated with @awaelchli

[mergify]

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

pls check again

[awaelchli]

Suggested change

	try:
	# add the attribute to the function but don't overwrite in case Trainer has already set it
	getattr(rank_zero_only, 'rank')
	except AttributeError:
	rank_zero_only.rank = 0
	# add the attribute to the function but don't overwrite in case Trainer has already set it
	rank_zero_only.rank = getattr(rank_zero_only, 'rank', 0)

I added the try catch block but this is probably more elegant :)