initialize device when pinning memory on this device, short circuit i… #145752
Conversation
This file contains hidden or 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
…s_pinned if device is not initialized
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/145752
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit ad816b7 with merge base 30dea84 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Co-authored-by: albanD <albandes@fb.com>
|
Hm no this actually initializes context on the 0th device (I think it shouldn't have, but I'll have to check) so won't work as is |
|
The behavior above is caused by pinning itself, not by initializing context, so this diff is a strict improvement over existing state. |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
The merge job was canceled or timed out. This most often happen if two merge requests were issued for the same PR, or if merge job was waiting for more than 6 hours for tests to finish. In later case, please do not hesitate to reissue the merge command |
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
mori360
pushed a commit
to mori360/pytorch
that referenced
this pull request
Feb 6, 2025
pytorch#145752) …s_pinned if device is not initialized Do not land RFC potential fix for pytorch#144687 Now `.is_pinned(device="cuda")` does not initialize device and thus doesn't poison the fork (but it complains about `device` arg being deprecated). To not need `device=` arg we'd need to fix get_accelerator to not initialize device. Pull Request resolved: pytorch#145752 Approved by: https://github.com/albanD Co-authored-by: albanD <albandes@fb.com>
BartlomiejStemborowski
added a commit
to BartlomiejStemborowski/pytorch
that referenced
this pull request
Mar 12, 2025
PR pytorch#145752 has added a check in the isPinnedPtr to check if a device is initialized before checking if the tensor is pinned. Also that PR has added a lazy initialization trigger when an at::empty is called with a pinned param set to true. However, when the tensor is firstly created and it is pinned in a separate call by calling pin_memory() function, lazy device init is not called so is_pinned returns always false. With this PR, the lazy initialization is moved to getPinnedMemoryAllocator function, thus it is assured that device is initialized before we pin a tensor.
This was referenced Mar 12, 2025
Closed
pytorchmergebot
pushed a commit
to BartlomiejStemborowski/pytorch
that referenced
this pull request
Mar 12, 2025
PR pytorch#145752 has added a check in the isPinnedPtr to check if a device is initialized before checking if the tensor is pinned. Also that PR has added a lazy initialization trigger when an at::empty is called with a pinned param set to true. However, when the tensor is firstly created and it is pinned in a separate call by calling pin_memory() function, lazy device init is not called so is_pinned returns always false. With this PR, the lazy initialization is moved to getPinnedMemoryAllocator function, thus it is assured that device is initialized before we pin a tensor.
pytorchmergebot
pushed a commit
that referenced
this pull request
Mar 13, 2025
…itialization. (#149033) PR #145752 has added a check in the isPinnedPtr to check if a device is initialized before checking if the tensor is pinned. Also that PR has added a lazy initialization trigger when an at::empty is called with a pinned param set to true. However, when the tensor is firstly created and it is pinned in a separate call by calling pin_memory() function, lazy device init is not called so is_pinned returns always false. With this PR, the lazy initialization is moved to getPinnedMemoryAllocator function, thus it is assured that device is initialized before we pin a tensor. Fixes #149032 @ngimel @albanD Pull Request resolved: #149033 Approved by: https://github.com/ngimel, https://github.com/albanD
pytorchbot
pushed a commit
that referenced
this pull request
Mar 14, 2025
…itialization. (#149033) PR #145752 has added a check in the isPinnedPtr to check if a device is initialized before checking if the tensor is pinned. Also that PR has added a lazy initialization trigger when an at::empty is called with a pinned param set to true. However, when the tensor is firstly created and it is pinned in a separate call by calling pin_memory() function, lazy device init is not called so is_pinned returns always false. With this PR, the lazy initialization is moved to getPinnedMemoryAllocator function, thus it is assured that device is initialized before we pin a tensor. Fixes #149032 @ngimel @albanD Pull Request resolved: #149033 Approved by: https://github.com/ngimel, https://github.com/albanD (cherry picked from commit 420a9be)
atalman
pushed a commit
that referenced
this pull request
Mar 17, 2025
…itialization. (#149183) [regression] Fix pin_memory() when it is called before device lazy initialization. (#149033) PR #145752 has added a check in the isPinnedPtr to check if a device is initialized before checking if the tensor is pinned. Also that PR has added a lazy initialization trigger when an at::empty is called with a pinned param set to true. However, when the tensor is firstly created and it is pinned in a separate call by calling pin_mem 7715 ory() function, lazy device init is not called so is_pinned returns always false. With this PR, the lazy initialization is moved to getPinnedMemoryAllocator function, thus it is assured that device is initialized before we pin a tensor. Fixes #149032 @ngimel @albanD Pull Request resolved: #149033 Approved by: https://github.com/ngimel, https://github.com/albanD (cherry picked from commit 420a9be) Co-authored-by: Bartlomiej Stemborowski <bstemborowskix@habana.ai>
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.
…s_pinned if device is not initialized
Do not land
RFC
potential fix for #144687
Now
.is_pinned(device="cuda")does not initialize device and thus doesn't poison the fork (but it complains aboutdevicearg being deprecated). To not needdevice=arg we'd need to fix get_accelerator to not initialize device.cc @malfet, @albanD