-
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
guard against None in pytorch get_xla_supported_devices #9572
Conversation
I'm actually not sure how to write a proper test for this. Seems like it would have to pull an XLA container. |
We have TPU CI tests in place, But for this, we would require to pull an XLA container with no TPUs attached. Could you please add a comment for this in the code? Do you think "the guard against None and return an empty list" should be added to the XLA's API itself? cc: @JackCaoG |
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.
Thanks for this! At the least I think add a todo
saying that this needs to be tested in a XLA supported container without TPUs!
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.
Blocking this merge for now. I have asked the XLA team if this change could instead be introduced on the pt/xla's side
@kaushikb11 so |
I feel like somebody might change |
I could try to change the pt/xla code to return an empty list, but
so I will prefer to leave the |
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 !
Thanks @ckchow for the contribution! We always look forward to improving our compatibility with TPUs! :) |
Codecov Report
@@ Coverage Diff @@
## master #9572 +/- ##
=======================================
- Coverage 93% 89% -4%
=======================================
Files 178 178
Lines 15652 15650 -2
=======================================
- Hits 14508 13899 -609
- Misses 1144 1751 +607 |
…#9572) Co-authored-by: Chris Chow <[email protected]> Co-authored-by: thomas chaton <[email protected]>
What does this PR do?
Guard against
None
when checking if TPU's are available. This happens (e.g.) if you use google's standard deep learning containers on CPU only machines.Fixes #9552
Does your PR introduce any breaking changes? If yes, please list them.
None
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 🙃