Repository navigation
test(tensilelite): normalize skinny TN problem metadata - #14885
Draft
ThanHenderson wants to merge 1 commit into
Draft
ThanHenderson wants to merge 1 commit into
ThanHenderson wants to merge 1 commit into
Conversation
github-actions
Bot
added
project: hipblaslt
project: hipsparselt
ci:hipsparselt-fast
labels
✅ 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. |
tony-davis
added a commit
that referenced
this pull request
Nov 2, 2026
ISSUE ID : #14887 Reverts #14281. #14281 added explicit defaults to the gfx950 Equality logic YAMLs, which breaks `test_skinny_tn_range_logic` in tensilelite for the BF16 and FP16 skinny-TN Range logic. The tensilelite CI was not triggered on #14281, so it merged with clean CI, and the failure is now blocking required tensilelite jobs on develop. Tracking in #14887. Revert check: the revert applies cleanly to current develop (git merge-tree dry run), no later commits touch the same files, and #14281 is not a TheRock bump. #14885 is a test-only fix-forward that can land independently so #14281 can be relanded later.
This was referenced Nov 2, 2026
Contributor
|
Hey Nathan! Thanks for jumping on this one so fast. I picked up the wider cleanup in #14891 (ROCM-33835), and I think it supersedes this PR. Your idea of comparing the effective |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (50.96%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #14885 +/- ##
========================================
Coverage 77.43% 77.43%
========================================
Files 2902 2902
Lines 489838 489845 +7
Branches 74139 74142 +3
========================================
+ Hits 379296 379304 +8
+ Misses 85575 85574 -1
Partials 24967 24967
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
tony-davis
added a commit
that referenced
this pull request
Nov 6, 2026
… check it on logic-only changes (#14891) JIRA ID : ROCM-33835 Also closes ROCM-33834 and AIHPBLAS-4196. ## Motivation Some TensileLite unit tests read the production hipBLASLt logic corpus directly and compared YAML across files, so their result depended on files the PR author and the test selector can't see. That's how #14281 broke `test_skinny_tn_range_logic`: it regenerated an Equality file with 14 explicit default keys, changed no values, and the test failed anyway (#14887). The same tests always skip in TheRock's installed-artifact lane, and nothing said so. ## Technical Details The rule I'm going for: a unit test doesn't read the corpus. Code checks use fixtures. Cross-file facts about the shipped data go in `ValidCorpusConsistency`, which `TensileLogic --check-all` runs in every kernel-generating build. What happened to each corpus reader: - `test_skinny_tn_range_logic`: the "Range file shares the Equality file's placeholder" check moved to a new `find_range_sibling_violations`, run by `--check-all`. It compares effective `ProblemType` after defaults are filled in (the same idea as #14885; thanks Nathan, and this supersedes it). The test keeps its single-file checks and compares against an inline plain TN type instead of a second file. - `test_generated_kernels_read_stagger_from_the_packed_argument`: reads verbatim copies of the two small Equality files under `Tests/unit/logic_yaml/staggeru/`. - `test_PlaceholderMerge.py`: the sibling-DeviceNames copy is gone, since `--check-all` already enforces it. The chip-ID arch lock walks `SUPPORTED_GFX` instead of the corpus, so it runs everywhere. - `test_solution_uid_uniqueness.py`: stays in its `library-uniqueness` component job, which now sets `HIPBLASLT_REQUIRE_LOGIC_CORPUS=1` so a missing corpus fails instead of skipping. Supporting pieces: - `Tests/unit/logic_corpus.py` is the one allowed way to reach the corpus. Tests that still need it carry `@needs_logic_corpus`, and every run prints `needs_logic_corpus: N ran, M skipped`. If the corpus is present but a named file is gone, the test fails rather than skips. - `test_no_logic_corpus_paths.py` fails any test that builds its own path from `__file__` into `amd_detail`, `rocblaslt` or `asm_full`. It flags all four original readers on develop. - `tensilelite/TESTING.md` has the audit table and updated gap rows. `TESTING.md` and `test_inventory.yaml` match. One surprise: a general "same-basename siblings must match" rule isn't true today. The corpus has 13 header and 6 `ProblemType` mismatches among Equality, GridBased and Origami siblings (gfx950 Equality says `CUCount: 256`, its Origami sibling says none). So the new check only holds Range files to their Equality sibling. I don't know whether the others are intended; it might be worth a look from someone on tuning. I didn't add a new TensileLogic flag. CI triggering on logic-only diffs stays with ROCM-33834. ## CI: checking the corpus on logic-only changes (ROCM-33834) The logic corpus is tuning data outside `tensilelite/`, so a PR that changes only logic YAML triggers no TensileLite lane. That's how #14281 merged with clean CI. The build-wired `--check-all` also only covers the architectures PR builds target (Linux gfx94X, gfx950, gfx125X and Windows gfx110X), so logic for gfx1151, gfx90a, gfx120X and others gets no check before merge. The second commit grows the `library-uniqueness` component lane into `Component CI: hipBLASLt Logic Corpus` (`component-ci-logic-corpus.yml`, renamed rather than added). It runs `tox -e logic-corpus` on any change under `Tensile/Logic/`, the `TensileLogic` validators, or the check script. That runs `scripts/run_tensile_logic_check.py --strict-known-bugs` for the architectures the PR's logic files declare, then every `@needs_logic_corpus` test with the corpus required. A validator or `known_bugs.yaml` change checks the whole corpus, as does a manual run. It rolls up to `Component CI Summary`, which the develop ruleset requires, so a failure blocks the merge. This also covers AIHPBLAS-4196 (a strict known-bugs lane). Cost and placement: it stays on `ubuntu-24.04` with a sparse checkout, no container. Locally a logic-only run took about 2.5 minutes (rocisa build 67 s, gfx1151 check 19 s, tests 42 s), and I'd guess the hosted runner adds a few minutes for the ROCm install and a slower rocisa build. The whole corpus takes about half an hour on four workers, which is why it isn't the default. This PR changes a validator, so its own CI runs the full sweep and will show the real number. I didn't fold this into the TensileLite coverage lane: that one triggers only on `tensilelite/**` and spends about 7 minutes on coverage, which tuning PRs don't need. ## Test Plan - Touched and adjacent unit tests with the corpus present, and from a copy of the tree with no corpus. - Swap in #14281's Equality file; run develop's test, the new test, and the validator. - Plant a divergence in a corpus copy; run the validator and `TensileLogic --check-all`. - `TensileLogic --check-all --architecture gfx950` on the real corpus. - `tox -e library-uniqueness`, plus the strict missing-corpus path. - pre-commit, and flake8 on the changed files. - `tox -e logic-corpus` scoped to gfx1151, plus negative checks: a bad gfx1151 solution, a Range/Equality divergence, a renamed Range file, a duplicated `SolutionUID`, and a stale known-bugs entry. - Path-filter simulation in `component_ci.py`, and actionlint on the new workflow. ## Test Result - With the corpus: 1125 passed, `needs_logic_corpus: 3 ran, 0 skipped`. - Without it: 246 passed, 3 skipped, and the summary line says so. - Develop's test fails both BF16 and FP16 cases on #14281's file (reproduces #14887). The new test and the validator pass on it. - The planted divergence is reported and `--check-all` exits 1. The real gfx950 corpus passes (84,953 solutions, 0 rejected). - library-uniqueness passes; with the corpus missing and strict set, it errors as intended. - I ran this on a CPU-only WSL box with an older ROCm nightly, so a few unrelated tests fail or crash locally (`test_every_custom_kernel_disassembles`, `test_occupancy_hip.py`). CI is the real check there. - Logic-corpus lane locally: passes in 2m24s scoped to gfx1151. All five negative cases exit non-zero (the stale entry only under `--strict-known-bugs`). A gfx950-only logic change triggers only the new lane. ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests. ## Risk level Low to medium. Most of it is tests, docs and CI config. Two product-side changes: a new check in `TensileLogic --check-all` (clean on today's corpus), and a CI lane that now gates logic-only PRs. Running ROCm setup on a hosted runner is new for this repo; if that's fiddly I'll split the CI commit into its own PR. --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: James Newling <james.newling@gmail.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tony-davis <7938412+tony-davis@users.noreply.github.com>
This branch has not been deployed
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.
JIRA ID : AIHPBLAS-5133
Motivation
#14281 added explicit defaults to gfx950 Equality logic, causing the BF16/FP16 skinny-TN Range metadata tests to fail on equivalent configurations.
Technical Details
Compare normalized ProblemType states and include MacDataTypeA/B in the BF16/FP16 datatype overrides.
Test Plan
Run
ROCISA_BACKEND=rocisa python -m pytest -q tensilelite/Tests/unit/test_CustomKernelMetadata.py -o addopts=''.Test Result
Both failures reproduced before the fix. The affected test module and pre-commit checks pass afterward. Checks with changed MX scaling and MAC datatypes still detect those differences.
Submission Checklist
Risk level
Low: test-only metadata comparison change.