Skip to content

[TESTS] Refactor tests to run on either the GPU or CPU - #6331

Merged
tqchen merged 1 commit into
apache:masterfrom
tkonolige:test_markers
Sep 2, 2020
Merged

[TESTS] Refactor tests to run on either the GPU or CPU#6331
tqchen merged 1 commit into
apache:masterfrom
tkonolige:test_markers

Conversation

@tkonolige

@tkonoligetkonolige commented Aug 24, 2020

Copy link
Copy Markdown
Contributor

Much of the time spent in testing is duplicated work between CPU and GPU test nodes. The main reason is that there is no way to control which TVM devices are enabled at runtime, so tests that use LLVM will run on both GPU and CPU nodes.

This patch adds an environment variable, TVM_TEST_DEVICES, which controls which TVM devices should be used by tests. Devices not in TVM_TEST_DEVICES can still be used, so tests must be careful to check that the desired device is enabled with tvm.testing.device_enabled or by enumerating all devices with tvm.testing.enabled_devices. All tests have been retrofitted with these checks.

This patch also provides the decorator @tvm.testing.gpu to mark a test as possibly using the gpu. Tests that require the gpu can use @tvm.testing.requires_gpu. Tests without these flags will not be run on GPU nodes.

@tkonolige
tkonoligeforce-pushed the test_markers branch 3 times, most recently from e64a748 to 324066aCompareAugust 24, 2020 19:09
tqchen
tqchen requested changes Aug 24, 2020
View reviewed changes
Comment threadpython/tvm/testing.py
Comment threadpython/tvm/testing.py Outdated
Comment threadpython/tvm/_ffi/runtime_ctypes.py Outdated
Comment threadpython/tvm/testing.py Outdated
Comment threadtests/python/topi/python/test_topi_conv2d_nhwc_winograd.py Outdated
Comment threadpython/tvm/testing.py Outdated
Comment threadtests/scripts/setup-pytest-env.sh Outdated
tqchen
tqchen requested changes Aug 24, 2020
View reviewed changes
Comment threadJenkinsfile Outdated
@tqchen

tqchen commented Aug 24, 2020

Copy link
Copy Markdown
Member

cc @zhiics@yzhliu@junrushao1994@merrymercy@ajtulloch@kparzysz-quic

@tqchen

tqchen commented Aug 24, 2020

Copy link
Copy Markdown
Member

It might also be helpful to send a quick RFC given that the change of test infra will affect quite a lot of people

@tkonolige

tkonolige commented Aug 24, 2020

Copy link
Copy Markdown
ContributorAuthor

I agree that an RFC could be useful, but maybe I could just add information to the docs instead?

zhiics
zhiics reviewed Aug 24, 2020
View reviewed changes

@zhiicszhiics left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nitpicks

Comment threadtests/python/contrib/test_random.py
Comment threadtests/python/relay/dyn/test_dynamic_op_level3.py Outdated
kparzysz-quic
kparzysz-quic reviewed Aug 25, 2020
View reviewed changes

@kparzysz-quickparzysz-quic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep in mind that eventually the mapping target_name -> device will no longer be sufficient. There can be several devices with different characteristics that may all fall under the same general target (e.g. gpu), and that any individual test should be able to specify required device properties.
The use of strings to pass targets around may be phased out at some point, please document somewhere the necessary updates to this scheme when that happens.

Comment threadpython/tvm/testing.py Outdated
Comment threadpython/tvm/testing.py Outdated
@tkonolige

tkonolige commented Aug 25, 2020

Copy link
Copy Markdown
ContributorAuthor

@kparzysz-quic I agree that using strings for targets here is pretty awkward. Is there a better way to check which device a target requires? I see that #6315 makes targets more structured, but I don't see a way to take a target string and get the associated device.

@kparzysz-quic

kparzysz-quic commented Aug 25, 2020

Copy link
Copy Markdown
Contributor

Currently there is no good way to switch away from str here, but the question is how much work would it take to allow, for example, two different 'cuda' devices to be available. I haven't analyzed the code well enough to see, but if you could outline somewhere what changes would be necessary, it would help the future transition.

@tkonolige

tkonolige commented Aug 26, 2020

Copy link
Copy Markdown
ContributorAuthor

@tqchen I think I've addressed all issues. Do I need to do something to have Jenkins use the new Jenkinsfile?

@tqchen

tqchen commented Aug 26, 2020

Copy link
Copy Markdown
Member

@tkonolige because uses the old Jenkinsfile (before it get merged). Please try to send another PR to add gpuonly_test that redirects to the normal integration test first, then we can tweak the scripts once that PR get merged

@tkonoligetkonolige mentioned this pull request Aug 26, 2020
Merged
@tkonolige
tkonoligeforce-pushed the test_markers branch 3 times, most recently from 89f2623 to 64265aeCompareAugust 28, 2020 00:21
@tkonolige

tkonolige commented Aug 28, 2020

Copy link
Copy Markdown
ContributorAuthor

@tqchen I'm getting a couple errors with CUDA initialization failing. I'm really sure of the cause, but it seems like it might have to do with forking.

@tkonolige
tkonoligeforce-pushed the test_markers branch 10 times, most recently from a5abf4f to f18f71bCompareSeptember 1, 2020 17:47
@tkonolige
tkonoligeforce-pushed the test_markers branch 2 times, most recently from ab0b4bc to bd404c6CompareSeptember 2, 2020 00:01
@tkonoligeTristan Konolige
[TESTS] Refactor tests to run on either the GPU or CPU.
574a7dd
Much of the time spent in testing is duplicated work between CPU and GPU
test nodes. The main reason is that there is no way to control which
TVM devices are enabled at runtime, so tests that use LLVM will run on
both GPU and CPU nodes.
This patch adds an environment variable, TVM_TEST_DEVICES, which
controls which TVM devices should be used by tests. Devices not in
TVM_TEST_DEVICES can still be used, so tests must be careful to check
that the desired device is enabled with `tvm.testing.device_enabled` or
by enumerating all devices with `tvm.testing.enabled_devices`. All
tests have been retrofitted with these checks.
This patch also provides the decorator `@tvm.testing.gpu` to mark a test
as possibly using the gpu. Tests that require the gpu can use
`@tvm.testing.requires_gpu`. Tests without these flags will not be run
on GPU nodes.
@tkonolige

tkonolige commented Sep 2, 2020

Copy link
Copy Markdown
ContributorAuthor

@tqchen All tests are passing now... can we merge?

@tqchen
tqchen merged commit b235e59 into apache:masterSep 2, 2020
@tqchen

tqchen commented Sep 2, 2020•
edited
Loading

Copy link
Copy Markdown
Member

Thanks @tkonolige@zhiics@kparzysz-quic !

@leandronleandron mentioned this pull request Sep 4, 2020
Closed
kevinthesun pushed a commit to kevinthesun/tvm that referenced this pull request Sep 17, 2020
@tkonolige@kevinthesun
[TESTS] Refactor tests to run on either the GPU or CPU. (apache#6331)
bc14291
Much of the time spent in testing is duplicated work between CPU and GPU
test nodes. The main reason is that there is no way to control which
TVM devices are enabled at runtime, so tests that use LLVM will run on
both GPU and CPU nodes.
This patch adds an environment variable, TVM_TEST_DEVICES, which
controls which TVM devices should be used by tests. Devices not in
TVM_TEST_DEVICES can still be used, so tests must be careful to check
that the desired device is enabled with `tvm.testing.device_enabled` or
by enumerating all devices with `tvm.testing.enabled_devices`. All
tests have been retrofitted with these checks.
This patch also provides the decorator `@tvm.testing.gpu` to mark a test
as possibly using the gpu. Tests that require the gpu can use
`@tvm.testing.requires_gpu`. Tests without these flags will not be run
on GPU nodes.
kevinthesun pushed a commit to kevinthesun/tvm that referenced this pull request Sep 18, 2020
@tkonolige@kevinthesun
[TESTS] Refactor tests to run on either the GPU or CPU. (apache#6331)
e09f320
Much of the time spent in testing is duplicated work between CPU and GPU
test nodes. The main reason is that there is no way to control which
TVM devices are enabled at runtime, so tests that use LLVM will run on
both GPU and CPU nodes.
This patch adds an environment variable, TVM_TEST_DEVICES, which
controls which TVM devices should be used by tests. Devices not in
TVM_TEST_DEVICES can still be used, so tests must be careful to check
that the desired device is enabled with `tvm.testing.device_enabled` or
by enumerating all devices with `tvm.testing.enabled_devices`. All
tests have been retrofitted with these checks.
This patch also provides the decorator `@tvm.testing.gpu` to mark a test
as possibly using the gpu. Tests that require the gpu can use
`@tvm.testing.requires_gpu`. Tests without these flags will not be run
on GPU nodes.
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Sep 18, 2020
@tkonolige@trevor-m
[TESTS] Refactor tests to run on either the GPU or CPU. (apache#6331)
2ceeea1
Much of the time spent in testing is duplicated work between CPU and GPU
test nodes. The main reason is that there is no way to control which
TVM devices are enabled at runtime, so tests that use LLVM will run on
both GPU and CPU nodes.
This patch adds an environment variable, TVM_TEST_DEVICES, which
controls which TVM devices should be used by tests. Devices not in
TVM_TEST_DEVICES can still be used, so tests must be careful to check
that the desired device is enabled with `tvm.testing.device_enabled` or
by enumerating all devices with `tvm.testing.enabled_devices`. All
tests have been retrofitted with these checks.
This patch also provides the decorator `@tvm.testing.gpu` to mark a test
as possibly using the gpu. Tests that require the gpu can use
`@tvm.testing.requires_gpu`. Tests without these flags will not be run
on GPU nodes.
@LunderbergLunderberg mentioned this pull request Jul 28, 2021
Merged
@Johnson9009Johnson9009 mentioned this pull request Aug 2, 2021
Merged
@LunderbergLunderberg mentioned this pull request Aug 20, 2021
Merged
@LunderbergLunderberg mentioned this pull request Jan 11, 2022
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zhiicszhiicszhiics left review comments
@tqchentqchentqchen approved these changes
+1 more reviewer
@kparzysz-quickparzysz-quickparzysz-quic left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

status: accepted

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tkonolige@tqchen@kparzysz-quic@zhiics