[ROCm] Enable/fix unit tests test_stream_args and test_event_args#82346
[ROCm] Enable/fix unit tests test_stream_args and test_event_args#82346alugorey wants to merge 1 commit intopytorch:masterfrom
Conversation
|
Hi @alugorey! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
🔗 Helpful links
✅ No Failures (0 Pending)As of commit 38fab50 (more details on the Dr. CI page): Expand to see more💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
2dcbf1d to
a564ca4
Compare
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
a564ca4 to
38fab50
Compare
|
@malfet Please review |
|
@pytorchbot merge |
|
@pytorchbot successfully started a merge job. Check the current status here |
|
Merge failed due to This PR is too stale; the last push date was more than 3 days ago. Please rebase and try again. |
Hmm, shouldn't those be guarded on a particular ROCM runtime version? |
|
@pytorchbot merge -f "[OTHER] This code doesn't change that often" |
|
@pytorchbot successfully started a merge job. Check the current status here |
|
Hey @alugorey. |
…2346) (#82346) Summary: ### Description Removed some stubbed out code that was necessary for ROCm builds to support JIT compilation of Event and Stream classes. Original motivation for the code to be stubbed out in the ROCm case was likely due to this pull request: #48020 In this PR, the include statement at the at the top of cuda.h was incorrectly pointed to aten/src/ATen/cuda/CUDAEvent.h when it should have been set to ATen/cuda/CUDAEvent.h. This error caused the hipification process of build_amd.py to not hipify this include statement correctly, causing errors. The include statement in question was subsequently fixed in the following commit: acd0729 This PR re-introduces the stubbed out code to the ROCm build and "unskips" the associated unit tests. ### Testing Note: bullets prepended by ROCm were tested on systems with AMD GPUs while the others were tested with NVIDIA GPUs. - apply commit - (ROCm)`python tools/amd_build/build_amd.py` - `python setup.py develop` - (ROCm)`PYTORCH_TEST_WITH_ROCM=1 python test/test_jit.py TestCUDA.test_event_args` - (ROCm)`PYTORCH_TEST_WITH_ROCM=1 python test/test_jit.py TestCUDA.test_stream_args` - `python test/test_jit.py TestCUDA.test_event_args` - `python test/test_jit.py TestCUDA.test_stream_args` - Confirm tests pass in all scenarios Pull Request resolved: #82346 Approved by: https://github.com/malfet Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/f1a135690731e04cc69795bd0efc2ab76e00a25d Reviewed By: kit1980 Differential Revision: D38359360 fbshipit-source-id: a32ba48dde3d9cc7a1a18deb94a4140933ab99e2
Description
Removed some stubbed out code that was necessary for ROCm builds to support JIT compilation of Event and Stream classes. Original motivation for the code to be stubbed out in the ROCm case was likely due to this pull request:
#48020
In this PR, the include statement at the at the top of cuda.h was incorrectly pointed to aten/src/ATen/cuda/CUDAEvent.h when it should have been set to ATen/cuda/CUDAEvent.h. This error caused the hipification process of build_amd.py to not hipify this include statement correctly, causing errors. The include statement in question was subsequently fixed in the following commit:
acd0729
This PR re-introduces the stubbed out code to the ROCm build and "unskips" the associated unit tests.
Testing
Note: bullets prepended by ROCm were tested on systems with AMD GPUs while the others were tested with NVIDIA GPUs.
python tools/amd_build/build_amd.pypython setup.py developPYTORCH_TEST_WITH_ROCM=1 python test/test_jit.py TestCUDA.test_event_argsPYTORCH_TEST_WITH_ROCM=1 python test/test_jit.py TestCUDA.test_stream_argspython test/test_jit.py TestCUDA.test_event_argspython test/test_jit.py TestCUDA.test_stream_args