-
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
[Onnx][UnitTests] Excluded additional onnx tests #8574
Conversation
@mbrookhart Can you review this, especially the addition of the cuda-specific excluded tests? |
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 confusion on parametarization, otherwise LGTM
pytest.skip() | ||
break | ||
@pytest.mark.parametrize("onnx_test", onnx_test_folders) | ||
def test_onnx_nodes(target, dev, onnx_test): |
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.
I'm not seeing how target and dev get into the test?
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.
This was a feature that I added in #8010 , where any unit test or fixture that accepts a target
or dev
argument is parametrized over all targets in tvm.testing.enabled_targets
. Basically, the same as the explicit pytest.mark.parametrize
that was there previously, or the tvm.testing.parametrize_targets
, but without needing each function to reproduce the same boilerplate. The target
parametrize is defined in tvm.testing._auto_parametrize_target
, and the dev
fixture based on it is in the conftest.py
.
- The onnx tests `test_basic_convinteger`, `test_convinteger_with_padding`, `test_range_float_type_positive_delta_expanded`, and `test_range_int32_type_positive_delta_expanded` don't run correctly on CUDA targets, so they are added to the exclusion. - Parametrized over the relative directory name, rather than the full directory name. This improves readability of the pytest output, and keeps the same parametrized test name across different python version. - Changed the target-specific skips to check the target kind, rather than the full target string.
Closing, this clean-up will be done along-side #8542, which was based on this. |
The onnx tests
test_basic_convinteger
,test_convinteger_with_padding
,test_range_float_type_positive_delta_expanded
, andtest_range_int32_type_positive_delta_expanded
don't run correctly on CUDA targets, so they are added to the exclusion.Parametrized over the relative directory name, rather than the full directory name. This improves readability of the pytest output, and keeps the same parametrized test name across different python version.
Changed the target-specific skips to check the target kind, rather than the full target string.