quick-fix for --gpus flag bug

[ananyahjha93]

What does this PR do?

Prevents setting self.on_gpu as True when --gpus flag is not passed by the user in the case where the following way is used to create the trainer

parser = Trainer.add_argparse_args(parser)
args = parser.parse_args()

trainer = Trainer.from_argparse_args(args)

Fixes #2669

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?

[codecov]

Codecov Report

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

@@          Coverage Diff           @@
##           master   #2674   +/-   ##
======================================
  Coverage      92%     92%           
======================================
  Files          74      74           
  Lines        6381    6382    +1     
======================================
+ Hits         5850    5851    +1     
  Misses        531     531           

[awaelchli]

I think the true fix should be to parse the gpus flag first and then set the on_gpu property, right?

[ananyahjha93]

@awaelchli In this case type takes a callable as a value in form Trainer._arg_default which then evaluates to the gpus flag values if something is passed to the script by the user. If the user does not include the --gpus flag, args.gpus refers to Trainer._arg_default callable.

https://github.com/PyTorchLightning/pytorch-lightning/blob/bugfix/gpus-flag/pytorch_lightning/trainer/trainer.py#L786

[awaelchli]

Yes I understand that. We also have a function that takes the user input and converts it to a list.
See distrib_parts.py::_parse_gpu_ids.
There, a callable results in None, which is what we want, right?
It happens later in the trainer init, so if you just move the on_gpu down there after it was parsed, it should be fine, no?
https://github.com/PyTorchLightning/pytorch-lightning/blob/62ce00f96c09de6d137c810921a6cd9e7b60aff5/pytorch_lightning/trainer/trainer.py#L531
Basically after here

[Borda]

yes, I would not pass the function at all, it makes unnecessary complex...

[ananyahjha93]

@awaelchli yeah, you are right, thanks for pointing this out.

@Borda what's your suggestion here, if not pass a function?

[williamFalcon]

i don't understand what this means

[ananyahjha93]

self.data_parallel_device_ids = _parse_gpu_ids(self.gpus)

_parse_gpu_ids returns None is self.gpus is callable, if you don't pass --gpus flag then self.gpus points to Trainer._arg_default

[awaelchli]

We could move the self.on_gpu = True if (gpus and torch.cuda.is_available()) else False found a few lines above
down here and simply write

self.on_gpu = True if (self.data_parallel_device_ids and torch.cuda.is_available()) else False

then there is no need for an extra if clause or a comment :)

[awaelchli]

really nice this is fixed, it was annoying with the default arg not properly set.

[mergify]

Great job! =)

[mergify]

Great job! =)

[Borda]

we shall re-count nb tests we are running as this was merged automatically even one test was failing...