Fix segfault in training unit tests #2929
Closed
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
✅ Deploy Preview for pytorch-fbgemm-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
This pull request was exported from Phabricator. Differential Revision: D60627636 |
|
This pull request was exported from Phabricator. Differential Revision: D60627636 |
sryap
added a commit
to sryap/FBGEMM
that referenced
this pull request
Aug 2, 2024
Summary: X-link: facebookresearch/FBGEMM#30 Pull Request resolved: pytorch#2929 Before this diff, there was a segmentation fault error (P1507485454) when running the SSD-TBE unit tests. It was caused by the premature tensor deallocation when the unit test invoked `set_cuda`. Since `set_cuda` is non-blocking asynchronous, the unit test must ensure that the input tensors are alive until `set_cuda` is complete. However, the unit test allocated an input tensor inside a for-loop (in a stack memory). The tensor was deallocated as soon as each for-loop iteration was done -- causing segmentation fault. This diff fixes the problem by making sure that the input tensor is alive until `set_cuda` is complete by moving the scope of the tensor outside of the for-loop and adding a proper synchronization. Reviewed By: duduyi2013 Differential Revision: D60627636
Summary: X-link: facebookresearch/FBGEMM#30 Pull Request resolved: pytorch#2929 Before this diff, there was a segmentation fault error (P1507485454) when running the SSD-TBE unit tests. It was caused by the premature tensor deallocation when the unit test invoked `set_cuda`. Since `set_cuda` is non-blocking asynchronous, the unit test must ensure that the input tensors are alive until `set_cuda` is complete. However, the unit test allocated an input tensor inside a for-loop (in a stack memory). The tensor was deallocated as soon as each for-loop iteration was done -- causing segmentation fault. This diff fixes the problem by making sure that the input tensor is alive until `set_cuda` is complete by moving the scope of the tensor outside of the for-loop and adding a proper synchronization. Reviewed By: duduyi2013 Differential Revision: D60627636
|
This pull request was exported from Phabricator. Differential Revision: D60627636 |
|
This pull request has been merged in 9cbf073. |
q10
pushed a commit
to q10/FBGEMM
that referenced
this pull request
Apr 10, 2025
Summary: Pull Request resolved: facebookresearch/FBGEMM#30 X-link: pytorch#2929 Before this diff, there was a segmentation fault error (P1507485454) when running the SSD-TBE unit tests. It was caused by the premature tensor deallocation when the unit test invoked `set_cuda`. Since `set_cuda` is non-blocking asynchronous, the unit test must ensure that the input tensors are alive until `set_cuda` is complete. However, the unit test allocated an input tensor inside a for-loop (in a stack memory). The tensor was deallocated as soon as each for-loop iteration was done -- causing segmentation fault. This diff fixes the problem by making sure that the input tensor is alive until `set_cuda` is complete by moving the scope of the tensor outside of the for-loop and adding a proper synchronization. Reviewed By: duduyi2013 Differential Revision: D60627636 fbshipit-source-id: a2016b9b23a154513bf851c07f6bdce4e7da70a6
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.
Summary:
Before this diff, there was a segmentation fault error (P1507485454)
when running the SSD-TBE unit tests. It was caused by the premature
tensor deallocation when the unit test invoked
set_cuda. Sinceset_cudais non-blocking asynchronous, the unit test must ensurethat the input tensors are alive until
set_cudais complete.However, the unit test allocated an input tensor inside a for-loop (in
a stack memory). The tensor was deallocated as soon as each for-loop
iteration was done -- causing segmentation fault.
This diff fixes the problem by making sure that the input tensor is
alive until
set_cudais complete by moving the scope of the tensoroutside of the for-loop and adding a proper synchronization.
Differential Revision: D60627636