Refactor GPUStatsMonitor to improve training speed

[rohitgr7]

What does this PR do?

Calls nvidia-smi once with all the required stats to monitor.
Follow up of #3008.

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 your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
• Did you make sure to update the documentation with your changes?
• Did you write any new necessary tests?
• Did you verify new and existing tests pass locally with your changes?
• If you made a notable change (that affects users), did you update the CHANGELOG?

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 #3257 into master will increase coverage by 3%.
The diff coverage is 86%.

@@           Coverage Diff           @@
##           master   #3257    +/-   ##
=======================================
+ Coverage      86%     89%    +3%     
=======================================
  Files          91      92     +1     
  Lines        8152    8311   +159     
=======================================
+ Hits         6989    7396   +407     
+ Misses       1163     915   -248     

[rohitgr7]

Questions:

• Should it be monitored for the GPUs that are actually used in the Trainer? For eg. user can have 10 GPUs but in the Trainer, only 2 are specified?
• Should row_log_interval and log_save_interval be considered here while logging?

[Borda]

• Should it be monitored for the GPUs that are actually used in the Trainer? For eg. user can have 10 GPUs but in the Trainer, only 2 are specified?

Yes

• Should row_log_interval and log_save_interval be considered here while logging?

Preferably

[Borda]

@rohitgr7 have we also renamed the files and class in #3008 so we need to maintain back-compatibility if it was already released in 0.9... check #3251 (comment)

[pep8speaks]

Hello @rohitgr7! Thanks for updating this PR.

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

Line 136:1: W293 blank line contains whitespace

Comment last updated at 2020-09-03 23:19:42 UTC

[Borda]

@rohitgr7 is it still wip?

[rohitgr7]

@Borda need to update this once #3251 get's merged.

[mergify]

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

[awaelchli]

the main speedup comes from less logging calls, is that correct?

[rohitgr7]

the main speedup comes from less logging calls, is that correct?

yep and also from single nvidia-smi call maybe.

[awaelchli]

good that the deprecation/backward compatibility is included now!

[SkafteNicki]

LGTM

[Red-Eyed]

It will not work on Windows

[Red-Eyed]

Do you consider to use pynvml instead?

[SkafteNicki]

I can get it to work on my windows machine. Just had to add nvidia-smi to PATH variable, and restart. On my computer it is stored in: C:\Program Files\NVIDIA Corporation\NVSMI

[Red-Eyed]

Yep, but this is something that user should do manually
maybe in that case, also add smth like so?

if any([shutil.which('C:/Program Files/NVIDIA Corporation/NVSMI/nvidia-smi') is None,
        shutil.which('nvidia-smi') is None])

?

[SkafteNicki]

That would work in most cases, as long as the user has install in default location.
Maybe it should just be clarified in the docs, that the user needs to be able to call nvidia-smi from terminal and hint what windows users should do?

[Red-Eyed]

or we can add it manually into PATH (os.environ["PATH"]), but i don't know is if a valid behavior