Disable non blocking to device with MPS #14368
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
j0rd1smit
requested review from
Borda,
tchaton,
carmocca,
williamFalcon,
awaelchli,
kaushikb11 and
rohitgr7
as code owners
j0rd1smit
changed the title
carmocca
reviewed
Aug 23, 2022
carmocca
added
community
carmocca
approved these changes
Aug 23, 2022
carmocca
changed the title
Codecov Report
@@ Coverage Diff @@
## master #14368 +/- ##
==========================================
- Coverage 79% 76% -3%
==========================================
Files 111 332 +221
Lines 7258 26894 +19636
==========================================
+ Hits 5740 20432 +14692
- Misses 1518 6462 +4944 |
justusschock
approved these changes
Aug 24, 2022
|
@j0rd1smit seems like you fixed it. I think it's reasonable to update the type, but I'd do so in another PR as this might require further typing changes for mypy. If you want to tackle that as well, feel free to do so :) |
|
@justusschock agreed that should be a separate PR. So guess this PR is ready. |
akihironitta
approved these changes
Aug 25, 2022
mergify
bot
added
ready
mergify
bot
added
ready
mergify
bot
added
ready
awaelchli
reviewed
Aug 26, 2022
rohitgr7
pushed a commit
that referenced
this pull request
Aug 27, 2022
* disable non-blocking for mps due to race condition bug * fixed typo * fixed: unknown mps device for non arm systems * Removed unrobust test case * moved _MPS_DEVICES such that we used in apply_func * Resolve circular dependencies * Comment rewording * changed torchElasticEnvironment to a global import * simplified if statement to blocking device type * Added change to CHANGELOG * Update src/pytorch_lightning/utilities/apply_func.py * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fixed mypy not detecting casting of device * Moved check into if statement to mainain original behavior Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> Co-authored-by: Justus Schock <12886177+justusschock@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
lexierule
pushed a commit
that referenced
this pull request
Aug 31, 2022
* disable non-blocking for mps due to race condition bug * fixed typo * fixed: unknown mps device for non arm systems * Removed unrobust test case * moved _MPS_DEVICES such that we used in apply_func * Resolve circular dependencies * Comment rewording * changed torchElasticEnvironment to a global import * simplified if statement to blocking device type * Added change to CHANGELOG * Update src/pytorch_lightning/utilities/apply_func.py * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fixed mypy not detecting casting of device * Moved check into if statement to mainain original behavior Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> Co-authored-by: Justus Schock <12886177+justusschock@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR ensures that the race condition bug in Pytorch (pytorch/pytorch#83015) does not affect lightning. As discussed with @justusschock in #13285 this PR disables non-blocking moves to MPS devices since it can result in tensors with different values after the moves to the MPS device. To verify it works now, the test case
test_data_is_not_changed_after_move_to_mps_devicehas been added.Fixes #13285
Does your PR introduce any breaking changes? If yes, please list them.
No.
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:
Did you have fun?
Yes, this was my first ever PR 😀. It was quite fun to do. However, since it is my first time, I hope I did everything OK. If not, let me know. I'm happy to learn.