Repository navigation
test(hipdnn): restore the quick RMSNorm runtime_epsilon case - #12932
Merged
Merged
Conversation
This was referenced Oct 2, 2026
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The targeted deletions match the existing sweep cases and leave all four support files valid.
Review effort: Balanced
Findings: None
What changed in this PR
Removes stale runtime_epsilon support claims that reference nonexistent RMSNorm sweep cases, resolving support-claim verification failures.
Changes:
- Removes the obsolete gfx1151/Windows claim from four quick RMSNorm variants.
- Preserves valid claims and canonical JSON formatting.
| File | Description |
|---|---|
quick/RMSNorm/Default/support.json |
Removes stale claim. |
quick/RMSNorm/Variant2/support.json |
Removes stale claim. |
quick/RMSNorm/Variant3/support.json |
Removes stale claim. |
quick/RMSNorm/Variant4/support.json |
Removes stale claim. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
|
I don't think we should drop these claims. I think we should restore the
Suggested fix:
|
#9600 added a manual runtime_epsilon case to quick/RMSNorm/{Default, Variant2,Variant3,Variant4}. It promotes epsilon to a runtime pass-by-value scalar and is the only RMSNorm test of that path. #12107 regenerated these sweeps from C++ test captures. The case has no C++ test behind it, so it was dropped, while the sibling support.json files kept its gfx1151/windows claim. verify_support_claims.py then reported "case id 'runtime_epsilon' not found in sweep.json" for all four bundles. Append the case to each sweep.json exactly as it was at c9702d9^. The four graph templates still have the same tensor names and uids, and each case's tensor patch targets that variant's epsilon tensor.
place_bundles.py rewrote each sweep.json from the captured C++ cases
only. Cases written by hand ("generator": "manual" in their metadata)
have no C++ test behind them, so a regeneration dropped them without a
message. That is how #12107 lost the quick RMSNorm runtime_epsilon case.
Before writing a sweep, read the existing sweep.json at that path and
append its manual cases unchanged. A manual case that no longer fits the
regenerated template (different tensor uid set, a tensor patch on a uid
the template lacks, an unresolved placeholder, or an id a captured case
now uses) is an error: that sweep and its template are left as they
were, the error is printed, and the run exits 1.
test_migration.py gains test_place_keeps_manual_cases. It fails on the
old script (the manual case is gone after the rerun) and passes now.
yassinsolim
force-pushed
the
users/ysoliman/hqs-rmsnorm-claims
branch
from
October 2, 2026 21:57
0c2db50 to
92488af
Compare
yassinsolim
changed the title
Member
Author
|
@bghimireamd Agreed, and done. The branch is force-pushed and no longer drops the claims.
That check flags two existing manual cases on develop: The restored case was not rerun on a GPU. |
Contributor
TheRock Submodule Bump ActivityNewest first
|
adickin-amd
added a commit
that referenced
this pull request
Oct 14, 2026
Conflicts in the three provider test_categories_integration.yaml files: take develop (#13198 re-tiered bundles and removed full_* from the comprehensive tier), which supersedes this branch's earlier change. Restore the RMSNorm runtime_epsilon claim groups in the four quick/RMSNorm support.json files: #12932 restored the case in the sweeps, so the claims are no longer stale. Update running-tests.md for the new comprehensive tier contents and the ASM_SDPA lane, and note HIPDNN_LOG_GRAPH_DIR as a graph source in adding-tests.md.
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.
Closes #12913
Motivation
verify_support_claims.pyfails on develop withcase id 'runtime_epsilon' not found in sweep.jsonforquick/RMSNorm/{Default,Variant2,Variant3,Variant4}. #9600 addedruntime_epsilonthere as a manual case. It makes epsilon a runtime pass-by-value scalar and is the only RMSNorm test of that path. #12107 regenerated these sweeps from C++ test captures withplace_bundles.py. The case has no C++ test behind it, so the regeneration dropped it, while thesupport.jsonfiles kept its gfx1151/windows claim. Details in #12913.Technical Details
Two commits:
runtime_epsiloncase to the foursweep.jsonfiles, copied unchanged fromc9702d99f61^(436 lines added, nothing else in the files changes). The claims stay as they are. The graph templates have the same tensor names and uids as before feat(integration-tests): (Re)generate test bundles for RMSNorm and RMSNorm fused activation #12107, and each case's tensor patch targets that variant's epsilon tensor (uid 3, 2, 1 and 2 for Default, Variant2, Variant3 and Variant4).migration-scripts/place_bundles.pykeeps the manual cases ("generator": "manual") of an existingsweep.jsonwhen it rewrites it. A manual case that no longer fits the regenerated template (different tensor uid set, a patch on a uid the template lacks, an unresolved placeholder, or an id a captured case now uses) makes the run exit 1 and leaves that sweep and its template unchanged. Documented inmigration-scripts/README.md.What could break: a
place_bundles.pyrun over a tree with a manual case that no longer fits now fails where it used to drop the case. On develop the check flags two:runtime_epsiloninquick/BatchnormBackward/Defaultandruntime_epsilon_momentuminquick/BatchnormPointwise/Default. Their tensor sets match theVariant2template of the same operation, notDefault. Neither has a support claim. This PR does not move them.Test Plan
Host only, no GPU.
verify_support_claims.verify_allon the bundle root at develop and on this branch.scripts/test_verify_support_claims.py.migration-scripts/test_migration.pywith the new and the oldplace_bundles.py. The new manual-case check run against every sweep on this branch that has manual cases.Test Result
verify_all: 4 errors on develop (the fourruntime_epsilonclaims), 0 on this branch.test_verify_support_claims.py: 28/28 passed.test_migration.py: 14/14 passed. With the oldplace_bundles.py, the newtest_place_keeps_manual_casesfails (the manual case is gone after the rerun) and the other 13 pass.Not run: the restored case on a GPU. Its claim records a pass on gfx1151/windows before #12107. It has not run since #12107 changed the templates (added
alignment, an explicitis_runtime_pass_by_value: falseandmin_required_engine_api_version).Submission Checklist