[CI] Run hipSPARSELt when hipBLASLt subtree changes#7514
Open
tony-davis wants to merge 5 commits into
Open
Conversation
Map projects/hipblaslt to also activate the sparselt optional stack so hipsparselt builds and tests with the blas TheRock job. Reload therock_matrix in unit tests to avoid cross-test project_map mutation; align miopen+rocwmma test with a single combined matrix entry. Co-authored-by: Cursor <cursoragent@cursor.com>
Adds a no-op CMake comment so the PR diff includes projects/hipblaslt (non-.md paths are not CI-skippable). Drop this commit before merge if you want a CI-only diff. Co-authored-by: Cursor <cursoragent@cursor.com>
This reverts commit 9279955.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an automatic CI dependency so hipBLASLt-only PR changes also trigger hipSPARSELt testing in TheRock matrix, addressing under-testing of a real downstream consumer.
Changes:
- Introduce
SUBTREE_EXTRA_MATRIX_PROJECTSmapping and apply it incollect_projects_to_runto inject extra optional matrix projects. - Reload module between tests to reset mutated module-level state, and add/adjust assertions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/scripts/therock_matrix.py | Adds extra-matrix mapping and a loop that augments the project set with optional projects (e.g. sparselt for hipblaslt). |
| .github/scripts/tests/therock_matrix_test.py | Adds setUp reload, asserts hipsparselt is included for hipblaslt-only subtrees, and updates miopen+rocwmma expectation to a single combined entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (69.24%) 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 #7514 +/- ##
===========================================
- Coverage 66.36% 59.74% -6.61%
===========================================
Files 1606 1730 +124
Lines 267985 320867 +52882
Branches 37395 48397 +11002
===========================================
+ Hits 177825 191695 +13870
- Misses 75219 113092 +37873
- Partials 14941 16080 +1139
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
eidenyoshida
pushed a commit
that referenced
this pull request
May 15, 2026
…hift, AssertKRingShiftTailWrapOnly, BAddrInterleave (#7513) ## Summary Downstream **hipSPARSELt** / **TensileCreateLibrary** builds broke after **[#7443](#7443 (*Manual revert KRingShift*) because **TensileLite** stopped understanding several **Assert\*** / **KRingShift** fields that were **still present** in checked-in **Tensile logic YAML**. This PR **removes those stale keywords** from the affected **hipBLASLt** and **hipSPARSELt** logic files so parsing matches the reverted **Python** behavior. ## Root cause (what broke) - **[#7443](#7443 removed handlers in `projects/hipblaslt/tensilelite/Tensile/Contractions.py` for keys such as **`AssertFree1DivByMT1LowbitGT1`**, **`AssertKRingShiftTailWrapOnly`**, and related **KRingShift** / **BAddrInterleave** plumbing. - Shipped **YAML** under **`projects/hipsparselt/.../Tensile/Logic/...`** (e.g. **gfx950** / **gfx942**) and **`projects/hipblaslt/.../Tensile/Logic/...`** (e.g. **gfx1250** GridBased) still contained those fields. - **TensileCreateLibrary** then hit **`RuntimeError: Unknown assertion key: AssertFree1DivByMT1LowbitGT1`** (and would be vulnerable to the next unknown key if only partially cleaned). So the regression was **YAML ↔ parser skew** after the revert, not the drop Docker image or venv. ## What this PR does - **hipSPARSELt:** strips **`AssertFree1DivByMT1LowbitGT1`**, **`KRingShift`**, **`AssertKRingShiftTailWrapOnly`**, **`BAddrInterleave`** from the **Equality** logic YAMLs under **`aquavanjaram/gfx942`** and **`gfx950`** (the paths exercised by **gfx942;gfx950** builds). - **hipBLASLt:** same cleanup on the **8** **`gfx1250` GridBased** logic YAMLs that still carried those keys. No change to kernel math here—this aligns **on-disk logic** with the **post-#7443** **TensileLite** parser. ## How we know it’s fixed - **Math CI:** **[hipblaslt-drop-build #117](https://math-ci.amd.com/job/image-builder/job/hipblaslt-drop-build/117/)** completed **SUCCESS** with a **`ROCM_LIBS_REF`** that includes this fix, exercising **hipBLASLt** + **hipSPARSELt** with **gfx942;gfx950** (full drop-style lane). - **This PR’s presubmit:** touches `projects/hipsparselt/**` (and **hipBLASLt** logic), so **TheRock** runs the **hipSPARSELt** / **sparselt** workstream for this change—not only **blas**—which directly exercises **TensileCreateLibrary** on the updated YAML. - **PR tag:** includes **`[project:hipsparselt]`** so **hipSPARSELt** is **built and tested** on this PR alongside **hipBLASLt**, not only after merge. ## CI coverage context (why #7443 slipped) - **[#7519](#7519 — documents that **hipBLASLt-only** PRs previously did **not** reliably pull **hipSPARSELt** into the **TheRock** matrix, so **#7443** could merge green while still breaking downstream **hipSPARSELt** / drop builds. - **[#7514](#7514 — follow-up to wire **`projects/hipblaslt/**`** so **`sparselt`** runs when **hipBLASLt** changes, closing that gap long-term (orthogonal to this YAML fix, but related). ## Checklist - [ ] Confirm no remaining **`AssertFree1DivByMT1LowbitGT1`** / stray **KRingShift** keys in the touched trees (`git grep` on the branch). - [ ] After merge, spot-check **TheRock** + optional **hipblaslt-drop-build** on **`develop`**. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
assistant-librarian Bot
pushed a commit
to ROCm/hipBLASLt
that referenced
this pull request
May 15, 2026
[hipblaslt][hipsparselt] Removed AssertFree1DivByMT1LowbitGT1, KRingShift, AssertKRingShiftTailWrapOnly, BAddrInterleave (#7513) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Downstream **hipSPARSELt** / **TensileCreateLibrary** builds broke after **[#7443](ROCm/rocm-libraries#7443 (*Manual revert KRingShift*) because **TensileLite** stopped understanding several **Assert\*** / **KRingShift** fields that were **still present** in checked-in **Tensile logic YAML**. This PR **removes those stale keywords** from the affected **hipBLASLt** and **hipSPARSELt** logic files so parsing matches the reverted **Python** behavior. ## Root cause (what broke) - **[#7443](ROCm/rocm-libraries#7443 removed handlers in `projects/hipblaslt/tensilelite/Tensile/Contractions.py` for keys such as **`AssertFree1DivByMT1LowbitGT1`**, **`AssertKRingShiftTailWrapOnly`**, and related **KRingShift** / **BAddrInterleave** plumbing. - Shipped **YAML** under **`projects/hipsparselt/.../Tensile/Logic/...`** (e.g. **gfx950** / **gfx942**) and **`projects/hipblaslt/.../Tensile/Logic/...`** (e.g. **gfx1250** GridBased) still contained those fields. - **TensileCreateLibrary** then hit **`RuntimeError: Unknown assertion key: AssertFree1DivByMT1LowbitGT1`** (and would be vulnerable to the next unknown key if only partially cleaned). So the regression was **YAML ↔ parser skew** after the revert, not the drop Docker image or venv. ## What this PR does - **hipSPARSELt:** strips **`AssertFree1DivByMT1LowbitGT1`**, **`KRingShift`**, **`AssertKRingShiftTailWrapOnly`**, **`BAddrInterleave`** from the **Equality** logic YAMLs under **`aquavanjaram/gfx942`** and **`gfx950`** (the paths exercised by **gfx942;gfx950** builds). - **hipBLASLt:** same cleanup on the **8** **`gfx1250` GridBased** logic YAMLs that still carried those keys. No change to kernel math here—this aligns **on-disk logic** with the **post-#7443** **TensileLite** parser. ## How we know it’s fixed - **Math CI:** **[hipblaslt-drop-build #117](https://math-ci.amd.com/job/image-builder/job/hipblaslt-drop-build/117/)** completed **SUCCESS** with a **`ROCM_LIBS_REF`** that includes this fix, exercising **hipBLASLt** + **hipSPARSELt** with **gfx942;gfx950** (full drop-style lane). - **This PR’s presubmit:** touches `projects/hipsparselt/**` (and **hipBLASLt** logic), so **TheRock** runs the **hipSPARSELt** / **sparselt** workstream for this change—not only **blas**—which directly exercises **TensileCreateLibrary** on the updated YAML. - **PR tag:** includes **`[project:hipsparselt]`** so **hipSPARSELt** is **built and tested** on this PR alongside **hipBLASLt**, not only after merge. ## CI coverage context (why #7443 slipped) - **[#7519](ROCm/rocm-libraries#7519 — documents that **hipBLASLt-only** PRs previously did **not** reliably pull **hipSPARSELt** into the **TheRock** matrix, so **#7443** could merge green while still breaking downstream **hipSPARSELt** / drop builds. - **[#7514](ROCm/rocm-libraries#7514 — follow-up to wire **`projects/hipblaslt/**`** so **`sparselt`** runs when **hipBLASLt** changes, closing that gap long-term (orthogonal to this YAML fix, but related). ## Checklist - [ ] Confirm no remaining **`AssertFree1DivByMT1LowbitGT1`** / stray **KRingShift** keys in the touched trees (`git grep` on the branch). - [ ] After merge, spot-check **TheRock** + optional **hipblaslt-drop-build** on **`develop`**.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
aledudek
pushed a commit
that referenced
this pull request
May 20, 2026
…hift, AssertKRingShiftTailWrapOnly, BAddrInterleave (#7513) ## Summary Downstream **hipSPARSELt** / **TensileCreateLibrary** builds broke after **[#7443](#7443 (*Manual revert KRingShift*) because **TensileLite** stopped understanding several **Assert\*** / **KRingShift** fields that were **still present** in checked-in **Tensile logic YAML**. This PR **removes those stale keywords** from the affected **hipBLASLt** and **hipSPARSELt** logic files so parsing matches the reverted **Python** behavior. ## Root cause (what broke) - **[#7443](#7443 removed handlers in `projects/hipblaslt/tensilelite/Tensile/Contractions.py` for keys such as **`AssertFree1DivByMT1LowbitGT1`**, **`AssertKRingShiftTailWrapOnly`**, and related **KRingShift** / **BAddrInterleave** plumbing. - Shipped **YAML** under **`projects/hipsparselt/.../Tensile/Logic/...`** (e.g. **gfx950** / **gfx942**) and **`projects/hipblaslt/.../Tensile/Logic/...`** (e.g. **gfx1250** GridBased) still contained those fields. - **TensileCreateLibrary** then hit **`RuntimeError: Unknown assertion key: AssertFree1DivByMT1LowbitGT1`** (and would be vulnerable to the next unknown key if only partially cleaned). So the regression was **YAML ↔ parser skew** after the revert, not the drop Docker image or venv. ## What this PR does - **hipSPARSELt:** strips **`AssertFree1DivByMT1LowbitGT1`**, **`KRingShift`**, **`AssertKRingShiftTailWrapOnly`**, **`BAddrInterleave`** from the **Equality** logic YAMLs under **`aquavanjaram/gfx942`** and **`gfx950`** (the paths exercised by **gfx942;gfx950** builds). - **hipBLASLt:** same cleanup on the **8** **`gfx1250` GridBased** logic YAMLs that still carried those keys. No change to kernel math here—this aligns **on-disk logic** with the **post-#7443** **TensileLite** parser. ## How we know it’s fixed - **Math CI:** **[hipblaslt-drop-build #117](https://math-ci.amd.com/job/image-builder/job/hipblaslt-drop-build/117/)** completed **SUCCESS** with a **`ROCM_LIBS_REF`** that includes this fix, exercising **hipBLASLt** + **hipSPARSELt** with **gfx942;gfx950** (full drop-style lane). - **This PR’s presubmit:** touches `projects/hipsparselt/**` (and **hipBLASLt** logic), so **TheRock** runs the **hipSPARSELt** / **sparselt** workstream for this change—not only **blas**—which directly exercises **TensileCreateLibrary** on the updated YAML. - **PR tag:** includes **`[project:hipsparselt]`** so **hipSPARSELt** is **built and tested** on this PR alongside **hipBLASLt**, not only after merge. ## CI coverage context (why #7443 slipped) - **[#7519](#7519 — documents that **hipBLASLt-only** PRs previously did **not** reliably pull **hipSPARSELt** into the **TheRock** matrix, so **#7443** could merge green while still breaking downstream **hipSPARSELt** / drop builds. - **[#7514](#7514 — follow-up to wire **`projects/hipblaslt/**`** so **`sparselt`** runs when **hipBLASLt** changes, closing that gap long-term (orthogonal to this YAML fix, but related). ## Checklist - [ ] Confirm no remaining **`AssertFree1DivByMT1LowbitGT1`** / stray **KRingShift** keys in the touched trees (`git grep` on the branch). - [ ] After merge, spot-check **TheRock** + optional **hipblaslt-drop-build** on **`develop`**. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
geomin12
reviewed
May 21, 2026
| # When these subtrees change, also activate the given optional matrix project so | ||
| # its additional_options merge into the parent job (e.g. hipSPARSELt depends on hipBLASLt). | ||
| SUBTREE_EXTRA_MATRIX_PROJECTS = { | ||
| "projects/hipblaslt": "sparselt", |
Collaborator
There was a problem hiding this comment.
FYI i have this implemented here: https://github.com/ROCm/TheRock/blob/7666949b442c6c7337e74577de103705a64ed0fa/math-libs/BLAS/CMakeLists.txt#L156-L158, where https://github.com/ROCm/TheRock/blob/main/test_tools/determine_rocm_test_dependencies.py will determine that test to run
I'm waiting on #7219 to land to get this included! we will also default to determine_rocm_test_dependencies.py script for future multi arch work
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.
Fixes #7519
What this PR does
SUBTREE_EXTRA_MATRIX_PROJECTSin.github/scripts/therock_matrix.py: when the changed subtree list includesprojects/hipblaslt, the matrix also includessparselt, so existingadditional_options["sparselt"]merges-DTHEROCK_ENABLE_SPARSE=ONandhipsparseltinto the same blas TheRock job as today’s hipSPARSELt-only PRs..github/scripts/tests/therock_matrix_test.py:importlib.reloadbetween tests (module-levelproject_mapis mutated), asserts hipsparselt for hipblaslt-only subtrees, and aligns the miopen+rocwmma expectation with one combined matrix row.Background: #7519. Same dependency-tree theme as TheRock #3491 (over-testing / unrelated blocking) vs. this change (under-testing a real downstream).
Risk analysis
hipsparseltinprojects_to_test).This PR’s presubmit TheRock job still expands to all subtrees (smoke) because it touches
.github/scripts/therock*(therock_configure_ci.pybehavior). That is not the matrix shape of a normal hipBLASLt code PR; narrow behavior is covered by pytest and by post-merge spot-checks on hipBLASLt-only diffs.Test plan
cd .github/scripts && python -m pytest tests/therock_matrix_test.py -v.github/**/therock*) — confirm TheRock blas matrix includes hipsparselt