Skip to content

Conversation

@Lunderberg
Copy link
Contributor

  • 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.

@Lunderberg
Copy link
Contributor Author

@mbrookhart Can you review this, especially the addition of the cuda-specific excluded tests?

Copy link
Contributor

@mbrookhart mbrookhart left a 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):
Copy link
Contributor

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?

Copy link
Contributor Author

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.
@Lunderberg
Copy link
Contributor Author

Closing, this clean-up will be done along-side #8542, which was based on this.

@Lunderberg Lunderberg closed this Aug 3, 2021
@Lunderberg Lunderberg deleted the onnx_tests branch August 23, 2021 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants