[ONNX][CI]Parametrize ONNX Unit tests

[mbrookhart]

#7438 Accidentally disabled the ONNX unit tests on CPU, and since then we've had a few regressions creep in. We've been attempting to enable the ONNX tests in a CPU only job at #8390 , but we've hit issues with pytorch in the CPU docker image.

This PR instead refactors the onnx test file to use the parametrize_targets flow.
it

1. adds that decorator to every test
2. inlines helper functions that are only used in one test as closures
3. removes the for loops over enabled targets so pytest will run every test on each target individually
4. removes -m gpu from the invocation of the pytest call in CI

This will re-enable a bunch of tests that have been disabled in CI and allow us to more easily skip tests that fail only on one target.

Thank you!

cc @electriclilies @Lunderberg @tkonolige @jwfromm @masahi

P.S. This conflicts somewhat with #8574 , but I really like the onnx node file changes in there.

[electriclilies]

Thanks for doing this @mbrookhart! Given the difficulty surrounding the docker image update (we've been trying for around a month to get the update through), I think that getting this refactor in is probably the fastest way to prevent further regressions.

[tkonolige]

This is a great change. Thanks for all the hard work. Is the plan to split CPU and GPU tests across the different nodes once we get onnx building on the cpu node?

Also, have you got a chance to see how much longer tests take with this change?

[csullivan]

👏 , thank you @mbrookhart!

[csullivan]

Suggested change

	@tvm.testing.parametrize_targets
	def test_forward_arg_min_max(target, dev):
	if "cuda" in target:
	pytest.skip("Fails on CUDA")
	@tvm.testing.known_failing_targets("cuda")
	@tvm.testing.parametrize_targets
	def test_forward_arg_min_max(target, dev):

Perhaps consider using known_failing_targets here and elsewhere

[mbrookhart]

I tried that on multiple tests, and they still ran on cuda and failed. This decorator apparently doesn't seem to work.

[Lunderberg]

Can you try using @tvm.testing.exclude_targets('cuda') instead? The known_failing_targets applies pytest.mark.xfail, which still runs a test to see if it succeeds, but doesn't count a failure of that test as a failure of the test suite. This makes sense if it is an assertion failure, but not if the test results in a segfault. The exclude_targets decorator removes those tests entirely from the ones being run.

[mbrookhart]

@tvm.testing.exclude_targets('cuda') fails in the same way, test runs, fails, and is reported as a failure.

[Lunderberg]

Follow-up, it looks like currently the @tvm.testing.parametrize_targets decorator doesn't respect the settings in known_failing_targets or exclude_targets. There's a slightly different code-path between the explicit parametrization and the auto-parametrization. This is resolved with #8542, which merges these two paths.

[mbrookhart]

This is a great change. Thanks for all the hard work. Is the plan to split CPU and GPU tests across the different nodes once we get onnx building on the cpu node?

I'm not planning on it unless we start an effort to do it for all frameworks, pytorch was segfaulting on CPU when I tried.

Also, have you got a chance to see how much longer tests take with this change?

It's a minimal change vs. running the tests locally before (perhaps 1-2 minutes of extra reference kernel calls). Unfortunately, many of these tests were disabled before, so this will up the CI time to re-enable them.

[mbrookhart]

Thanks @tkonolige @Lunderberg @jroesch @electriclilies @csullivan!

I've synced with @Lunderberg, we've decided to merge this and he will rebase #8542 and include @tvm.testing.known_failing_targets("cuda") on that PR which fixes the inconsistencies with the current API.