Repository navigation
[MIOpen] Remove MHA tests, fix BN test categorization for CI - #8122
Merged
Merged
Conversation
MHA (Multi-Head Attention) was never released and is being removed. This removes the test files as a first step. Large batchnorm tests were incorrectly categorized as Smoke, causing them to run in every standard CI run (~17 minutes on gfx1151 Windows). These are moved to Full so they only run in full/nightly test suites. The standard YAML category now excludes all Full/GPU_BN*Large* patterns. Also fixes a dataset swap bug in bn_infer.cpp where GPU_BNInferSmall2D_FP32 was using Network2DLarge (taking ~29s) and GPU_BNInferLarge2D_FP32 was using Network2DSmall. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
BradPepersAMD
force-pushed
the
users/brpepers/test-cleanup
branch
from
June 5, 2026 20:59
e3abb66 to
8e591ef
Compare
BrianHarrisonAMD
approved these changes
Jun 5, 2026
BrianHarrisonAMD
left a comment
BrianHarrisonAMD
left a comment
Contributor
There was a problem hiding this comment.
LGTM.
Ideally we land the MHA removal as well.
JonathanLichtnerAMD
approved these changes
Jun 5, 2026
JonathanLichtnerAMD
left a comment
JonathanLichtnerAMD
left a comment
Contributor
There was a problem hiding this comment.
LGTM, but we probably want to clean up the Full BN tests from the quick category
Remove Full/-prefixed BN Large tests from the quick category since quick should only contain Smoke/ tests. Remove duplicate specific BFP16/FP16 excludes from standard that are already covered by broader wildcard patterns. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
davidd-amd
pushed a commit
that referenced
this pull request
Jun 8, 2026
## Summary - Remove MHA (Multi-Head Attention) test files — the feature was never released and is being removed - Move large batchnorm tests from `Smoke` to `Full` prefix so they no longer run in standard CI (~17 minutes saved on gfx1151 Windows) - Fix dataset swap bug where `GPU_BNInferSmall2D_FP32` was using `Network2DLarge` data and `GPU_BNInferLarge2D_FP32` was using `Network2DSmall` data - Update `test_categories.yaml` to remove MHA patterns and add `Full/GPU_BN*Large*` excludes to the `standard` category - Update CMakeLists.txt and gfx11 GPU exclusion patterns to match the `Smoke` → `Full` prefix change ## Risk Assessment **MHA test removal (low risk):** MHA was never released. The test files are deleted but the implementation code remains for a separate cleanup. No functional code is changed, only test infrastructure. **BN Smoke→Full reclassification (low risk):** Large BN tests move from running on every PR to only running in full/nightly suites. Small BN tests continue to run in standard CI for coverage. If a regression affects only large tensor batchnorm, it won't be caught until a nightly run. This is an acceptable tradeoff — the large tests were taking ~17 minutes of every standard CI run. **BN Infer dataset swap fix (medium risk):** `GPU_BNInferSmall2D_FP32` was running `Network2DLarge` (39 test cases, ~29s) and `GPU_BNInferLarge2D_FP32` was running `Network2DSmall`. This swaps them to match the fixture names. The "Small" fixture will now run faster with fewer test cases. The "Large" fixture (now `Full` prefix) will run the larger dataset but only in full test suites. This changes what data gets tested in standard CI for this specific fixture — it will test small network configs instead of large ones in standard runs. ## Test plan - [x] Verify build succeeds (MHA files removed; glob-based discovery should just skip them) - [x] Run standard CI (`ctest -L standard`) and confirm no `*Large*BN*` or `*Mha*` tests appear - [x] Run full CI (`ctest -L full`) and confirm Large BN tests DO appear - [x] Confirm `GPU_BNInferSmall2D_FP32` runs significantly faster (~few seconds vs ~29s) - [x] Verify YAML is well-formed (validated via `yaml.safe_load`) - [x] Spot-check that no other tests were accidentally excluded from standard CI 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4 <noreply@anthropic.com>
1 of 5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SmoketoFullprefix so they no longer run in standard CI (~17 minutes saved on gfx1151 Windows)GPU_BNInferSmall2D_FP32was usingNetwork2DLargedata andGPU_BNInferLarge2D_FP32was usingNetwork2DSmalldatatest_categories.yamlto remove MHA patterns and addFull/GPU_BN*Large*excludes to thestandardcategorySmoke→Fullprefix changeRisk Assessment
MHA test removal (low risk):
MHA was never released. The test files are deleted but the implementation code remains for a separate cleanup. No functional code is changed, only test infrastructure.
BN Smoke→Full reclassification (low risk):
Large BN tests move from running on every PR to only running in full/nightly suites. Small BN tests continue to run in standard CI for coverage. If a regression affects only large tensor batchnorm, it won't be caught until a nightly run. This is an acceptable tradeoff — the large tests were taking ~17 minutes of every standard CI run.
BN Infer dataset swap fix (medium risk):
GPU_BNInferSmall2D_FP32was runningNetwork2DLarge(39 test cases, ~29s) andGPU_BNInferLarge2D_FP32was runningNetwork2DSmall. This swaps them to match the fixture names. The "Small" fixture will now run faster with fewer test cases. The "Large" fixture (nowFullprefix) will run the larger dataset but only in full test suites. This changes what data gets tested in standard CI for this specific fixture — it will test small network configs instead of large ones in standard runs.Test plan
ctest -L standard) and confirm no*Large*BN*or*Mha*tests appearctest -L full) and confirm Large BN tests DO appearGPU_BNInferSmall2D_FP32runs significantly faster (~few seconds vs ~29s)yaml.safe_load)🤖 Generated with Claude Code