[ROCm] re-enable tensorexpr and test_openmp #81367

arindamroy-eng · 2022-07-12T22:57:00Z

The following tests are being re-enabled for ROCm:

test_openmp.py
TestTensorExprPyBind tests in test_tensorexpr_pybind.py

facebook-github-bot · 2022-07-12T22:57:09Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/81367
✖️ Python docs build was skipped
✖️ C++ docs build was skipped
❓Need help or want to give feedback on the CI? Visit our office hours

✅ No Failures (0 Pending)

As of commit a801d90 (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.

Click here to manually regenerate this comment.

arindamroy-eng · 2022-07-13T02:15:12Z

@pruthvistony needs merge

pruthvistony · 2022-07-13T19:00:09Z

@arindamroy-eng ,
Check if the change is causing a problem for the libtorch_cpu.so build, due to llvm path changes.
What is the result from your local testing?

arindamroy-eng · 2022-07-18T21:18:08Z

find /opt/ -name libtorch_cpu.so

/opt/conda/lib/python3.7/site-packages/torch/lib/libtorch_cpu.so

The test_tensorexpr runs fine. It actually the test_mobile_nnc, which not a python file, has problem finding the libtorch_cpu.so as its not in its default LD path.

echo $LD_LIBRARY_PATH

/opt/ompi/lib:/opt/rocm/lib:/usr/local/lib

export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/opt/conda/lib/python3.7/site-packages/torch/lib/

/opt/conda/lib/python3.7/site-packages/torch/bin/test_mobile_nnc --gtest_output=xml:/test_mobile_nnc.xml
Note: Google Test filter = -_CUDA:*_MultiCUDA

[==========] Running 6 tests from 2 test suites.

[----------] Global test environment set-up.

arindamroy-eng · 2022-07-19T15:27:35Z

@priyaramani Can you help out in what should be added to the aot_test in .jenkins/pytorch/tesh.sh.

In the linuxfocal build, the aot test is passing, but it is writing its result to /, hence in the next step its failing:

/opt/conda/lib/python3.7/site-packages/torch/bin/test_mobile_nnc --gtest_output=xml:/test_mobile_nnc.xml
........
[ FATAL ] /var/lib/jenkins/workspace/third_party/googletest/googletest/src/gtest.cc:190:: Unable to open file "/test_mobile_nnc.xml"
69065
.jenkins/pytorch/test.sh: line 329: 42736 Aborted (core dumped) LD_LIBRARY_PATH="$LD_LIBRARY_PATH":/opt/conda/lib/python3.7/site-packages/torch/lib/ "$TORCH_BIN_DIR"/test_mobile_nnc --gtest_output=xml:$TEST_REPORTS_DIR/test_mobile_nnc.xml

arindamroy-eng · 2022-07-26T19:34:50Z

The failing test is in torch dynamo.
test_bce_with_logits_has_correct_forward_grad (main.TestNN)
File "/opt/conda/lib/python3.7/site-packages/torch/fx/graph_module.py", line 267, in call
75247
return super(self.cls, obj).call(*args, **kwargs) # type: ignore[misc]
75248
File "/opt/conda/lib/python3.7/site-packages/torch/nn/modules/module.py", line 1186, in _call_impl
75249
return forward_call(*input, **kwargs)
75250
TypeError: forward() takes 2 positional arguments but 3 were given
75251

This is unrelated to the PR.
@bdhirsh

arindamroy-eng · 2022-08-05T05:54:26Z

@pruthvistony @jeffdaily @jithunnair-amd This can be merged now.

jithunnair-amd · 2022-08-05T06:04:12Z

@malfet Who would be the right person to review this PR?

jeffdaily

Please revert the following commits:

Then please apply this patch:

diff --git a/.jenkins/pytorch/test.sh b/.jenkins/pytorch/test.sh
index b476d25250..432656c6af 100755
--- a/.jenkins/pytorch/test.sh
+++ b/.jenkins/pytorch/test.sh
@@ -322,6 +322,9 @@ test_libtorch() {

 test_aot_compilation() {
   echo "Testing Ahead of Time compilation"
+  ln -sf "$TORCH_LIB_DIR"/libc10* "$TORCH_BIN_DIR"
+  ln -sf "$TORCH_LIB_DIR"/libtorch* "$TORCH_BIN_DIR"
+
   if [ -f "$TORCH_BIN_DIR"/test_mobile_nnc ]; then "$TORCH_BIN_DIR"/test_mobile_nnc --gtest_output=xml:$TEST_REPORTS_DIR/test_mobile_nnc.xml; fi
   # shellcheck source=test/mobile/nnc/test_aot_compile.sh
   if [ -f "$TORCH_BIN_DIR"/aot_model_compiler_test ]; then source test/mobile/nnc/test_aot_compile.sh; fi

This follows the precedent elsewhere in test.sh to symlink the libs prior to running cpp tests.

The reason it had failed for rocm was due to the order of tests in the test.sh file. Targets other than rocm were symlinking as part of their steps, and those other tests were executed prior to test_aot_compilation() but rocm skipped them and therefore skipped the symlinking. And since test_aot_compilation() didn't do its own symlinking, it would fail for rocm.

.jenkins/pytorch/test.sh

arindamroy-eng · 2022-08-31T05:10:57Z

@pytorchmergebot please merge

pytorch-bot · 2022-08-31T05:10:59Z

❌ 🤖 pytorchbot command failed:

@pytorchbot: error: argument command: invalid choice: 'please' (choose from 'merge', 'revert', 'rebase', 'label')

usage: @pytorchbot [-h] {merge,revert,rebase,label} ...

Try @pytorchbot --help for more info.

malfet · 2022-09-13T21:19:11Z

@pytorchbot rebase

pytorchmergebot · 2022-09-13T21:21:18Z

@pytorchbot successfully started a rebase job. Check the current status here

By setting this path, TestTensorExprPyBind tests in test_tensorexpr_pybind.py will be enabled for ROCM Signed-off-by: Arindam Roy <rarindam@gmail.com>

Signed-off-by: Arindam Roy <rarindam@gmail.com>

When LLVM is being enabled for ROCM, it seems that the test_mobile_nnc does not have the libtorch_cpu.so in its LD_LIBRARY_PATH. Adding this path temorarily to run the binary. Signed-off-by: Arindam Roy <rarindam@gmail.com>

This test is for mobile device, and is throwing unrelated errors once llvm is being enabled. Hence disabling for ROCM. Signed-off-by: Arindam Roy <rarindam@gmail.com>

This reverts commit 60e170a.

This reverts commit a4947e5.

This follows the precedent elsewhere in test.sh to symlink the libs prior to running cpp tests. The reason it had failed for rocm was due to the order of tests in the test.sh file. Targets other than rocm were symlinking as part of their steps, and those other tests were executed prior to test_aot_compilation() but rocm skipped them and therefore skipped the symlinking. And since test_aot_compilation() didn't do its own symlinking, it would fail for rocm. Signed-off-by: Arindam Roy <rarindam@gmail.com>

pytorchmergebot · 2022-09-13T21:21:23Z

Successfully rebased ROCM_ReEnable_Tensorexpr onto refs/remotes/origin/master, please pull locally before adding more changes (for example, via git checkout ROCM_ReEnable_Tensorexpr && git pull --rebase)

pytorch-bot · 2022-09-13T21:21:28Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/81367

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures, 3 Pending

As of commit fcf09f1:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

malfet · 2022-09-13T21:33:11Z

@pytorchbot merge -g

pytorchmergebot · 2022-09-13T21:34:39Z

@pytorchbot successfully started a merge job. Check the current status here.
The merge job was triggered with the green (-g) flag. This means that your change will be merged once all checks on your PR have passed (ETA: 0-4 Hours). If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

github-actions · 2022-09-14T00:41:55Z

Hey @arindamroy-eng.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

The following tests are being re-enabled for ROCm: - test_openmp.py - TestTensorExprPyBind tests in test_tensorexpr_pybind.py Pull Request resolved: #81367 Approved by: https://github.com/jeffdaily, https://github.com/malfet

arindamroy-eng requested a review from a team as a code owner July 12, 2022 22:57

pytorch-bot bot added the module: rocm AMD GPU support for Pytorch label Jul 12, 2022

facebook-github-bot added the cla signed label Jul 12, 2022

pytorchbot added the open source label Jul 12, 2022

arindamroy-eng changed the title ~~RROCM re enable tensorexpr and test_openmp~~ ROCM: Re enable tensorexpr and test_openmp Jul 13, 2022

pruthvistony added the ciflow/trunk Trigger trunk jobs on your pull request label Jul 13, 2022

bdhirsh added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jul 20, 2022

jeffdaily requested changes Aug 29, 2022

View reviewed changes

jeffdaily changed the title ~~ROCM: Re enable tensorexpr and test_openmp~~ [ROCm] re-enable tensorexpr and test_openmp Aug 29, 2022

jeffdaily approved these changes Aug 29, 2022

View reviewed changes

jeffdaily reviewed Aug 29, 2022

View reviewed changes

.jenkins/pytorch/test.sh Show resolved Hide resolved

malfet approved these changes Sep 13, 2022

View reviewed changes

arindamroy-eng added 6 commits September 13, 2022 21:21

ROCM: Set proper LLVM path for RCOM builds

f6593f1

By setting this path, TestTensorExprPyBind tests in test_tensorexpr_pybind.py will be enabled for ROCM Signed-off-by: Arindam Roy <rarindam@gmail.com>

ROCM: Renable test_openmp for ROCM

83a18c9

Signed-off-by: Arindam Roy <rarindam@gmail.com>

ROCM:Add libtorch_cpu.so path for aot_mobile_nnc

62bec43

When LLVM is being enabled for ROCM, it seems that the test_mobile_nnc does not have the libtorch_cpu.so in its LD_LIBRARY_PATH. Adding this path temorarily to run the binary. Signed-off-by: Arindam Roy <rarindam@gmail.com>

ROCM: Disbale aot test for rocm

32a7fc3

This test is for mobile device, and is throwing unrelated errors once llvm is being enabled. Hence disabling for ROCM. Signed-off-by: Arindam Roy <rarindam@gmail.com>

Revert "ROCM: Disbale aot test for rocm"

6874c11

This reverts commit 60e170a.

Revert "ROCM:Add libtorch_cpu.so path for aot_mobile_nnc"

1d99e30

This reverts commit a4947e5.

arindamroy-eng and others added 2 commits September 13, 2022 21:21

set TEST_REPORTS_DIR for test_aot_compilation()

fcf09f1

pytorchmergebot force-pushed the ROCM_ReEnable_Tensorexpr branch from a801d90 to fcf09f1 Compare September 13, 2022 21:21

pytorchmergebot added the Merged label Sep 14, 2022

pytorchmergebot closed this in a185dc2 Sep 14, 2022

[ROCm] re-enable tensorexpr and test_openmp #81367

[ROCm] re-enable tensorexpr and test_openmp #81367

Uh oh!

Conversation

arindamroy-eng commented Jul 12, 2022 • edited by jeffdaily Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Jul 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

✅ No Failures (0 Pending)

Uh oh!

arindamroy-eng commented Jul 13, 2022

Uh oh!

pruthvistony commented Jul 13, 2022

Uh oh!

arindamroy-eng commented Jul 18, 2022

Uh oh!

arindamroy-eng commented Jul 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arindamroy-eng commented Jul 26, 2022

Uh oh!

arindamroy-eng commented Aug 5, 2022

Uh oh!

jithunnair-amd commented Aug 5, 2022

Uh oh!

jeffdaily left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arindamroy-eng commented Aug 31, 2022

Uh oh!

pytorch-bot bot commented Aug 31, 2022

Uh oh!

malfet commented Sep 13, 2022

Uh oh!

pytorchmergebot commented Sep 13, 2022

Uh oh!

pytorchmergebot commented Sep 13, 2022

Uh oh!

pytorch-bot bot commented Sep 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/81367

✅ No Failures, 3 Pending

Uh oh!

malfet commented Sep 13, 2022

Uh oh!

pytorchmergebot commented Sep 13, 2022

Uh oh!

github-actions bot commented Sep 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

arindamroy-eng commented Jul 12, 2022 •

edited by jeffdaily

Loading

facebook-github-bot commented Jul 12, 2022 •

edited

Loading

arindamroy-eng commented Jul 19, 2022 •

edited

Loading

pytorch-bot bot commented Sep 13, 2022 •

edited

Loading