Skip to content

Workaround for running full instead of standard - #5766

Merged
amd-hsivasun merged 2 commits into
developfrom
users/aalabsi-amd/update-categories-for-standard
Mar 24, 2026
Merged

amd-hsivasun merged 2 commits into
developfrom
users/aalabsi-amd/update-categories-for-standard

Conversation

@aalabsi-amd

Copy link
Copy Markdown
Contributor

Motivation

Unblock miopen CI since we don't maintain full yet, our tests intended for PRs are in standard.
Will address why PRs won't pick up standard in another PR.

Technical Details

copying standard tests to full

Test Plan

miopen runs the selected suites meant for PRs and passing.
No inclusions of tests that are outside standard such as: Full/GPU_LSTM_FP64

Test Result

Submission Checklist

@JonathanLichtnerAMD JonathanLichtnerAMD 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.

LGTM assuming we go with this approach vs reverting plus bumping the rock hash

- "*"
test_patterns: *positive_patterns

exclude:

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.

Can we remove this duplicate exclude as well?

@amd-hsivasun
amd-hsivasun merged commit 2f19806 into develop Mar 24, 2026
2 of 3 checks passed
@amd-hsivasun
amd-hsivasun deleted the users/aalabsi-amd/update-categories-for-standard branch March 24, 2026 19:27
- "Full/GPU_ConvGrpActivInfer3D_BFP16*"
- "Full/GPU_ConvGrpActivInfer3D_FP32*"
- "Full/GPU_ConvGrpActivInfer3D_FP16*"
- "Smoke/GPU_UnitTestConvSolverHipImplicitGemmV4R1Fwd_BFP16.ConvHipImplicitGemmV4R1Fwd/0"

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.

This one's missing a wildcard. I believe the intention is to disable all BFP16-Fwd tests. This instantiation shares a test case generator with several others, so it should be Smoke/GPU_UnitTestConvSolverHipImplicitGemmV4R1Fwd_BFP16.ConvHipImplicitGemmV4R1Fwd* to immunize us in the off chance someone adds more shapes.

Ref:

// Smoke tests
INSTANTIATE_TEST_SUITE_P(Smoke,
GPU_UnitTestConvSolverHipImplicitGemmV4R1Fwd_FP16,
testing::Combine(testing::Values(GetTestParamsFP16()),
testing::Values(miopenConvolutionAlgoImplicitGEMM),
testing::ValuesIn(GetConvTestCases(miopenHalf))));
INSTANTIATE_TEST_SUITE_P(Smoke,
GPU_UnitTestConvSolverHipImplicitGemmV4R1Fwd_BFP16,
testing::Combine(testing::Values(GetTestParamsBFP16()),
testing::Values(miopenConvolutionAlgoImplicitGEMM),
testing::ValuesIn(GetConvTestCases(miopenBFloat16))));

Comment on lines 286 to 287
# Only exclude explicitly disabled tests
- "*DISABLED*"

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.

The DISABLED filter should be removed--we shouldn't be automatically filtering DISABLED tests. Gtest provides the --gtest_also_run_disabled_tests flag for debugging purposes, but --gtest_filter takes precedence. This can cause false positives since it is easy to overlook this element inside the very long filter.

Example: unary_tensor_ops.cpp instantiates 10 suites, each having 55 tests, and two of the 10 are DISABLED:

Normal behavior:

miopen/build# bin/test_unary_tensor_ops
...
[==========] 440 tests from 10 test suites ran. (18395 ms total)
[  PASSED  ] 440 tests.

  YOU HAVE 110 DISABLED TESTS

2*55=110, so as expected.
Manually re-enabled:

miopen/build# bin/test_unary_tensor_ops  --gtest_also_run_disabled_tests
...
110 FAILED TESTS

Note: they are DISABLED for good reason :)
Filtered:

...miopen/build# bin/test_unary_tensor_ops --gtest_filter=-<many other filters>:*DISABLED*:<many other filters>
...
[==========] 440 tests from 10 test suites ran. (2558 ms total)
[  PASSED  ] 440 tests.

No mention of disabled tests.
Filtered and manually re-enabled:

miopen/build# bin/test_unary_tensor_ops --gtest_filter=-<many other filters>:*DISABLED*:<many other filters> --gtest_also_run_disabled_tests
...
[==========] 440 tests from 10 test suites ran. (1631 ms total)
[  PASSED  ] 440 tests.

Still no mention of the DISABLED tests, so the user may be tricked into thinking they ran and passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants