[ONNX] Enable GPU in ONNX importer tests #7438

electriclilies · 2021-02-10T21:17:27Z

In the onnx test, we were setting target and ctx to llvm and cpu every time we ran the TVM version of the op, regardless of the target and ctx actually passed in. The result is that the accuracy of TVM GPU outputs were never checked against ONNX.

The purpose of this PR is to see which of the tests in tests/python/frontend/onnx/test_forward.py break after enabling GPU.

mbrookhart · 2021-02-10T21:27:18Z

Definitely a needed improvement, we'll see what it uncovers.

masahi · 2021-02-11T11:11:29Z

oops indeed there are some problems with gpu test

[WIP] Fixes for GPU ONNX tests

…ged. Matt or myself will fix later but we don't have time right now.

masahi · 2021-03-18T05:03:47Z

tests/scripts/task_python_frontend.sh

@@ -31,6 +31,9 @@ find . -type f -path "*.pyc" | xargs rm -f
 # Rebuild cython
 make cython3

+# Only run GPU enabled tests on GPU
+export PYTEST_ADDOPTS="-m gpu $PYTEST_ADDOPTS"


Are we sure we want this? I think this is skipping a lot of tests that were previously running.

It seems there are many tests that don't use @tvm.testing.uses_gpu.

tvm/tests/python/frontend/pytorch/test_object_detection.py

Lines 93 to 94 in 96b0981

def test_detection_models():

Probably we should use targets option in verify_with_ort_with_inputs for now

tvm/tests/python/frontend/onnx/test_forward.py

Line 126 in 3beec22

targets=None,

See for example

tvm/tests/python/frontend/onnx/test_forward.py

Lines 3421 to 3423 in 3beec22

verify_with_ort_with_inputs(

model, [indata], targets=["llvm"], dtype="int64", use_vm=True, opset=9

)

(we should test this function on GPU, btw)

Right now, all the tests are running on GPU, which causes the argmin / argmax onnx test to fail. The ONNX test file uses @tvm.testing.uses_gpu to indicate whether we should run tests on GPU or not, but right now those decorators don't actually do anything, which is definitely not good. I think we should try to move towards enabling the @tvm.testing.uses_gpu in the long run.

What if I set PYTEST_ADDOPTS before the onnx tests, and reset it after? Since the file uses the @tvm.testing.uses_gpu a lot, I think we should either remove the decorators or let them actually do something.

Overall, I think that hardcoding targets is bad because it's not easy to see -- that's what caused none of the ONNX tests to be run on GPU in the first place.

ok, surprised to hear that use_gpu doesn't do anything. Let's make it

PYTEST_ADDOPTS="-m gpu $PYTEST_ADDOPTS" run_pytest cython python-frontend-onnx tests/python/frontend/onnx

This should only modify env vars when running the onnx test.

Also, can you go through TODO(mbrookhart) in the onnx frontend test and add use_gpus? There are about 10-15 of them.

tvm/tests/python/frontend/onnx/test_forward.py

Line 277 in 3beec22

# TODO(mbrookhart): enable once VM supports heterogenous execution

Doh! I thought I had removed all of those comments, but I must have only done it for the dynamic test files and missed the onnx file.

I can open a separate PR to remove these.

I already did it!

masahi · 2021-03-22T21:04:26Z

@electriclilies Can you take a look at the CI issue?

electriclilies · 2021-03-22T21:17:30Z

@masahi I forgot to disable the dynamic version of the batch matmul test on cuda, it should be fixed now.

masahi · 2021-03-27T00:33:21Z

@electriclilies Please try kick again, the flaky issue should be fixed now

electriclilies · 2021-03-30T06:44:36Z

@masahi It's green! Can you merge? :)

masahi · 2021-03-30T07:38:52Z

Thanks @electriclilies @mbrookhart this is merged!

* remove hardcoded target and ctx * fix c-codgen for floating point mod * MDisable onnx gpu test for argmin / argmax so we can get this fix merged. Matt or myself will fix later but we don't have time right now. * lint * fix black * Add flag to task_python_frontend.sh to only run GPU enabled tests on GPU * black again * Enable GPU for test_nonzero * Respond to comments * Don't test batch matmul on CUDA * Turn cuda off for dynamic batch matmul test * Fix task script * Flaky test * another flaky test Co-authored-by: mbrookhart <[email protected]>

remove hardcoded target and ctx

1595841

mbrookhart requested review from mbrookhart and masahi February 10, 2021 21:18

mbrookhart and others added 7 commits February 12, 2021 09:54

fix c-codgen for floating point mod

43668a9

Merge pull request #3 from mbrookhart/onnx_gpu_tests

8655ce5

[WIP] Fixes for GPU ONNX tests

MDisable onnx gpu test for argmin / argmax so we can get this fix mer…

c02d9fb

…ged. Matt or myself will fix later but we don't have time right now.

lint

371faff

fix black

fcbc54e

Add flag to task_python_frontend.sh to only run GPU enabled tests on GPU

dc4b083

black again

2f60c43

masahi reviewed Mar 18, 2021

View reviewed changes

electriclilies added 3 commits March 18, 2021 10:27

Enable GPU for test_nonzero

b58dc24

Respond to comments

e765689

Don't test batch matmul on CUDA

e583e9d

mbrookhart approved these changes Mar 19, 2021

View reviewed changes

Turn cuda off for dynamic batch matmul test

e6e5cf0

electriclilies added 3 commits March 23, 2021 12:44

Fix task script

4f5a41c

Merge

9f31e99

Flaky test

dabc571

electriclilies added 2 commits March 29, 2021 11:39

another flaky test

96bf1ba

Merge remote-tracking branch 'upstream/main' into onnx_gpu_enabled

9268af2

masahi approved these changes Mar 30, 2021

View reviewed changes

masahi merged commit 1c2555a into apache:main Mar 30, 2021

electriclilies deleted the onnx_gpu_enabled branch March 30, 2021 18:58

mbrookhart mentioned this pull request Aug 2, 2021

[ONNX][CI]Parametrize ONNX Unit tests #8621

Merged

junrushao mentioned this pull request Nov 1, 2021

Apache TVM v0.8 Release Note Candidate #9416

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ONNX] Enable GPU in ONNX importer tests #7438

[ONNX] Enable GPU in ONNX importer tests #7438

electriclilies commented Feb 10, 2021

mbrookhart commented Feb 10, 2021

masahi commented Feb 11, 2021

masahi Mar 18, 2021 •

edited

Loading

masahi Mar 18, 2021

electriclilies Mar 18, 2021 •

edited

Loading

electriclilies Mar 18, 2021

masahi Mar 18, 2021 •

edited

Loading

mbrookhart Mar 18, 2021

mbrookhart Mar 18, 2021

electriclilies Mar 18, 2021

masahi commented Mar 22, 2021

electriclilies commented Mar 22, 2021 •

edited

Loading

masahi commented Mar 27, 2021

electriclilies commented Mar 30, 2021

masahi commented Mar 30, 2021

	verify_with_ort_with_inputs(
	model, [indata], targets=["llvm"], dtype="int64", use_vm=True, opset=9
	)

[ONNX] Enable GPU in ONNX importer tests #7438

[ONNX] Enable GPU in ONNX importer tests #7438

Conversation

electriclilies commented Feb 10, 2021

mbrookhart commented Feb 10, 2021

masahi commented Feb 11, 2021

masahi Mar 18, 2021 • edited Loading

Choose a reason for hiding this comment

masahi Mar 18, 2021

Choose a reason for hiding this comment

electriclilies Mar 18, 2021 • edited Loading

Choose a reason for hiding this comment

electriclilies Mar 18, 2021

Choose a reason for hiding this comment

masahi Mar 18, 2021 • edited Loading

Choose a reason for hiding this comment

mbrookhart Mar 18, 2021

Choose a reason for hiding this comment

mbrookhart Mar 18, 2021

Choose a reason for hiding this comment

electriclilies Mar 18, 2021

Choose a reason for hiding this comment

masahi commented Mar 22, 2021

electriclilies commented Mar 22, 2021 • edited Loading

masahi commented Mar 27, 2021

electriclilies commented Mar 30, 2021

masahi commented Mar 30, 2021

masahi Mar 18, 2021 •

edited

Loading

electriclilies Mar 18, 2021 •

edited

Loading

masahi Mar 18, 2021 •

edited

Loading

electriclilies commented Mar 22, 2021 •

edited

Loading