-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[OpenCL] Enable OpenCL for GPU tests #12490
Conversation
This PR enables OpenCL test in CI. For that:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your PR! Please, rebase it to the latest mainline. And one question. Will we run our cpp opencl tests in the CI?
CMakeLists.txt
Outdated
@@ -798,4 +798,4 @@ if(${SUMMARIZE}) | |||
print_summary() | |||
endif() | |||
|
|||
dump_options_to_file("${TVM_ALL_OPTIONS}") | |||
dump_options_to_file("${TVM_ALL_OPTIONS}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No meaningful changes. Please revert it.
… tests are skipped in the environment script
@echuraev I found better solutions to exclude tests, now they are excluded in CI scripts. Please check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Several minor comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
cc: @masahi, @csullivan, @TejashShah, @driazati |
|
||
# forked is needed because the global registry gets contaminated | ||
TVM_TEST_TARGETS="${TVM_RELAY_TEST_TARGETS:-llvm;cuda}" \ | ||
run_pytest ctypes ${TVM_INTEGRATION_TESTSUITE_NAME}-relay tests/python/relay | ||
|
||
# OpenCL texture test. Deselected specific tests that fails in CI | ||
TVM_TEST_TARGETS="${TVM_RELAY_OPENCL_TEXTURE_TARGETS:-opencl}" \ | ||
run_pytest ctypes ${TVM_INTEGRATION_TESTSUITE_NAME}-opencl-texture tests/python/relay/opencl_texture --deselect=tests/python/relay/opencl_texture/test_conv2d_nchw_texture.py::test_residual_block |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of deselecting here can you:
- File a GitHub issue with the test name and some context on why it fails with OpenCL
- Move the skipping to the test itself via
pytest.mark.skipIf
and be sure to add areason="See <link to apache/tvm GitHub issue>"
?
That might make it so you don't need to change this script at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the main thing here is that this test works well locally on adreno devices, so you don’t want to skip it. But in CI environment due to we have nvidia device so different libOpenCL, this test doesn't work properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally we want to exclude some tests only for nvidia devices, but I couldn't find the proper way to exclude tests for nvidia. So maybe there is some way to exclude tests by making sure it doesn't run on nvidia?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can do whatever logic is necessary in the condition for the skipIf
so that it's only skipped in the specific cases you want (e.g. here maybe just skipIf(tvm.testing.utils.IS_IN_CI, reason="...")
). Also in tvm/testing/utils.py
take a look at requires_cuda
or uses_gpu
as they may already do what you need
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added skipif instead of deselect. Thanks for the suggestion!
@tvm-bot rerun |
# forked is needed because the global registry gets contaminated | ||
TVM_TEST_TARGETS="${TVM_RELAY_TEST_TARGETS:-llvm;cuda}" \ | ||
run_pytest ctypes ${TVM_INTEGRATION_TESTSUITE_NAME}-relay tests/python/relay | ||
|
||
# OpenCL texture test. Deselected specific tests that fails in CI | ||
TVM_TEST_TARGETS="${TVM_RELAY_OPENCL_TEXTURE_TARGETS:-opencl}" \ | ||
run_pytest ctypes ${TVM_INTEGRATION_TESTSUITE_NAME}-opencl-texture tests/python/relay/opencl_texture |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one more thing, won't this get run as part of test/python/relay
in the run_pytest
above this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a large number of tests in tests/python/realy/
for OpenCL do not run, for various reasons, so I took out the texture tests separately so as not to mark relay tests for some special occasions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests/
script changes look good to me
cc @masahi @junrushao, it needs a merge trigger! |
* Add opencl target in test build script * Fix fp16 test and compile test for opencl * fix lint * Fix relay OpenCL texture tests * Fix lint * Enable relay OpenCL tests * Fix opencl relay texture tests * fix lint * Remove OpenCL gtest variable * Fix unbound variable * Skip tests that are not supported in CI * fix lint * Add path for opencl gtest directory * Fix opencl gtests include directory * Enable OpenCL googletest. Fix bug in opencl timer test * testing fix for build cpp tests * update googletest git version for opencl tests build * update cmakelist * Update CMakeList * Update CMakeList * Disable opencl googletests * update Opecnl.cmake * fix Opecnl.cmake * Apply comments. Remove xfail decerator for opencl tests. Now specific tests are skipped in the environment script * minor code changes * apply comments * apply comment * skip test in ci by decorator * fix pytest skipif warnings * Fix skipif for opencl gtests
Enable OpenCL support for GPU tests.