-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix gpus -1 for CPU #8766
fix gpus -1 for CPU #8766
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8766 +/- ##
=======================================
- Coverage 93% 92% -1%
=======================================
Files 169 169
Lines 14077 14079 +2
=======================================
- Hits 13045 12934 -111
- Misses 1032 1145 +113 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 😃 Maybe update a CPU test to use gpus=-1
to show that it is working
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice to include this change in the changelog :)
I would also like to suggest not classifying this as bugfix :) |
Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !
@@ -581,6 +579,8 @@ def parallel_devices(self) -> List[Union[torch.device, int]]: | |||
# https://github.com/PyTorchLightning/pytorch-lightning/issues/3169 | |||
if isinstance(self.tpu_cores, int): | |||
devices = list(range(self.tpu_cores)) | |||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you there ? tpu_cores=[5] is also a valid value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so far reading code not...
cc: @kaushikb11 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com>
What does this PR do?
the
gpus=-1
shall be robust even if you do not have any GPUDoes your PR introduce any breaking changes? If yes, please list them.
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃