Repository navigation
fix(ci): register hipblaslt/tensilelite as a nested subtree + longest-prefix-match - #11785
Conversation
…-prefix-match pr_detect_changed_subtrees.py find_matched_subtrees() truncated every changed file path to exactly 2 segments (category/name), so a change confined to projects/hipblaslt/tensilelite was indistinguishable from a change to projects/hipblaslt-proper. Switch to longest-prefix-match (mirrors the same fix landing in TheRock's configure_external_repo_ci.py), and register projects/hipblaslt/tensilelite in repos-config.json as a nested subtree (auto_subtree_pull/push=false: it is not an independently synced subtree, just a change-detection-only entry; hipblaslt own entry already covers its actual sync). This is a change-detection precision fix only. It surfaces a more specific selector (projects/hipblaslt/tensilelite) to the Multi-Arch CI pipeline, where TheRock determine_rocm_test_dependencies.py (see companion TheRock PR) now knows how to route it correctly: a TensileLite-only change retests hipblaslt/rocblas/hipblas transitively, without forcing the reverse (a hipblaslt-proper change no longer needs to retest TensileLite). The new, more specific selector would otherwise silently fall out of the legacy single-arch CI hardcoded subtree_to_project_map and ROCJITSU_RACE_CHECK_SUBTREES / SUBTREE_EXTRA_MATRIX_PROJECTS lookups in therock_matrix.py (both keyed on the literal projects/hipblaslt string), so this also adds the matching projects/hipblaslt/tensilelite entries there -- same bucket, same rocjitsu selection, same sparselt activation as a hipblaslt-proper change, preserving legacy behavior exactly. Co-authored-by: Cursor <cursoragent@cursor.com>
✅ 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. |
There was a problem hiding this comment.
🟢 Approval recommended
The changes are cohesive, tested, and align with the stated CI-selection intent, with only a minor optional hardening suggestion noted in review comments.
Pull request overview
This PR fixes CI subtree change-detection so changes under projects/hipblaslt/tensilelite/** are attributed to the more specific nested subtree (instead of collapsing into projects/hipblaslt), and updates legacy single-arch TheRock CI mapping so the new selector preserves existing test/bucket behavior.
Changes:
- Register
projects/hipblaslt/tensileliteas a nested subtree in.github/repos-config.jsonfor change-detection targeting (with sync automation disabled). - Update
.github/scripts/pr_detect_changed_subtrees.pyto use longest-prefix matching against registered subtrees. - Extend legacy TheRock matrix lookups and add unit tests to ensure
tensilelite-only changes behave likehipblasltchanges in the legacy path (bucket, rocjitsu race-check, and sparselt activation).
File summaries
| File | Description |
|---|---|
.github/scripts/therock_matrix.py |
Adds projects/hipblaslt/tensilelite handling to legacy mappings/sets to preserve prior CI selection behavior. |
.github/scripts/tests/therock_matrix_test.py |
Adds coverage asserting nested TensileLite subtree behaves like hipBLASLt in the legacy matrix path. |
.github/scripts/tests/test_pr_detect_changed_subtrees.py |
Adds tests for nested-vs-parent attribution (longest-prefix behavior). |
.github/scripts/pr_detect_changed_subtrees.py |
Switches subtree attribution from fixed 2-segment truncation to longest registered-prefix matching. |
.github/repos-config.json |
Registers hipblaslt/tensilelite as a distinct (nested) subtree for more precise change detection. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Addresses Copilot review feedback on PR #11785: entry.name is treated as a filesystem path/filename by some automation (e.g. pr_merge_sync_patches.py builds Path(tmpdir) / entry.name and f"{entry.name}.patch"), so a slash in name is a latent trap. Move the nesting into category instead (projects/hipblaslt) and keep name slash-free (tensilelite). The matched prefix (category/name) is unchanged: projects/hipblaslt/tensilelite. Co-authored-by: Cursor <cursoragent@cursor.com>
geomin12
left a comment
There was a problem hiding this comment.
unit tests will help for confirmation
also, can we enable single arch ci and disable multi-arch via labels to show this works as expected? https://github.com/ROCm/rocm-libraries/blob/develop/docs/ci_behavior_manipulation.md
|
Good call on both. The tests are already in here, just further down in the diff: I just added |
Adds a no-op comment to README.md, confined to projects/hipblaslt/tensilelite/, to confirm this PR's detection logic resolves it to the tensilelite subtree and still selects hipblaslt/rocblas/hipblas for retest. Will be reverted before merge. Co-authored-by: Cursor <cursoragent@cursor.com>
…detection" This reverts commit ff0c52b.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #11785 +/- ##
===========================================
- Coverage 69.42% 69.42% -0.00%
===========================================
Files 2828 2828
Lines 470113 470113
Branches 69417 69417
===========================================
- Hits 326343 326342 -1
Misses 120066 120066
- Partials 23704 23705 +1
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
## Problem PR #11667 added a translation layer (`multi_arch_test_projects.py` + `therock_matrix.py`'s `shared/*` entries) so Multi-Arch CI could map `shared/*` monorepo subtrees (e.g. `shared/tensile`, `shared/origami`) to the right TheRock test projects, back when TheRock's own selector had no notion of these paths. TheRock's `test_tools/determine_rocm_test_dependencies.py` now natively resolves all five `shared/*` paths (confirmed already in the pinned ref, `df3d451a`), so the rocm-libraries-side pre-resolution step is redundant: raw `shared/*` paths can now pass straight through to TheRock's selector, exactly like every `projects/*` path already does. Tracked in #11784 (the remaining follow-up item from that issue), following #11785. ## Changes - **Deleted** `.github/scripts/multi_arch_test_projects.py` and its test file. This script's only caller was `therock-multi-arch-ci.yml`'s "Select multi-arch test projects" step. - **`therock-multi-arch-ci.yml`**: removed that step; `test_projects` now comes straight from `steps.configure.outputs.changed_projects`. Trimmed the now-unused `ci_utils.py`/`therock_matrix.py` sparse-checkout entries from the same job. - **`therock_matrix.py`'s `shared/*` entries in `subtree_to_project_map` are intentionally left untouched** -- they're still required by `therock_configure_ci.py` (the opt-in `ci:single-arch` legacy path), which never calls TheRock's native selector. ## Behavior delta (verified against the exact pinned TheRock commit, `df3d451a`) Ran `determine_rocm_test_dependencies.py` locally for all five `shared/*` paths, before (today's `multi_arch_test_projects.py` output) vs. after (native selector, default per-component levels, no CLI override): | Changed path | Before (legacy) | After (native) | Delta | |---|---|---|---| | `shared/origami` | hipblas, hipblaslt, origami, rocblas, tensilelite | hipblas, hipblaslt, hipblasltprovider, hipdnn-samples, hipsolver, hipsparse, hipsparselt, miopen, miopenprovider, origami, rocalution, rocblas, rocsolver, rocsparse, rocwmma, tensilelite | superset -- safe | | `shared/stinkytofu` | hipblas, hipblaslt, rocblas, tensilelite | hipblas, hipblaslt, hipblasltprovider, hipdnn-samples, hipsolver, hipsparse, hipsparselt, miopen, miopenprovider, rocalution, rocblas, rocsolver, rocsparse, rocwmma, tensilelite | superset -- safe | | `shared/mxdatagenerator` | hipblas, hipblaslt, rocblas, rocroller, tensilelite | hipblas, hipblaslt, hipblasltprovider, hipdnn, hipdnn-samples, hipsolver, hipsparse, hipsparselt, miopen, miopenprovider, rocalution, rocblas, rocroller, rocsolver, rocsparse, rocwmma, tensilelite | superset -- safe | | `shared/rocroller` | rocroller | hipblaslt, hipdnn, miopen, rocroller | superset -- safe | | `shared/tensile` | hipblas, hipblaslt, rocblas, tensilelite | hipblas, hipsolver, miopen, rocalution, rocblas, rocsolver, rocsparse, rocwmma | **narrows**: drops hipblaslt, tensilelite; adds hipsolver, miopen, rocalution, rocsolver, rocsparse, rocwmma | **On the `shared/tensile` narrowing:** checked `test_tools/therock_consumer_graph.json` -- `rocblas`'s consumers do not include `hipblaslt` (the edge runs the other way: `hipblaslt` consumes `rocblas`, not vice versa), so `hipblaslt`/`tensilelite` don't architecturally depend on `shared/tensile` changes. Accepting this as a correctness fix rather than papering over it with a manual addition, per architecture. ## Testing - `pytest .github/scripts/tests` -- 144 passed, 8 xfailed (pre-existing, unrelated) - `pre-commit run` on both changed files -- clean - Local simulation above against the exact pinned TheRock commit (`df3d451a`), not just current TheRock `main` -- default per-component walk levels, no `--level` override ## Notes Editing `therock-multi-arch-ci.yml` matches TheRock's own "run everything" fallback trigger patterns (and rocm-libraries' own), so this PR's live CI can't validate the underlying per-path selection logic on itself -- same caveat as #11785. The table above is the primary evidence. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. Co-authored-by: Cursor <cursoragent@cursor.com>
TheRock Submodule Bump ActivityNewest first
|
## Summary Test selection returns **no tests** for a change confined to TensileLite, because the nested subtree path `projects/hipblaslt/tensilelite` has no entry in `_EXTERNAL_SUBTREE_ALIASES`. This adds the alias. ## Motivation ROCm/rocm-libraries#11785 taught rocm-libraries' change detection to report `projects/hipblaslt/tensilelite` (longest-prefix match) instead of collapsing TensileLite files into `projects/hipblaslt`. `determine_rocm_test_dependencies.py` has no alias for that string, so it falls through to the `projects/` strip, produces the non-existent graph key `hipblaslt/tensilelite`, warns, and selects nothing — while exiting `0`. Verified against `df3d451` (the commit rocm-libraries pins today): ```console $ python test_tools/determine_rocm_test_dependencies.py \ --changed-projects projects/hipblaslt/tensilelite Warning: unrecognized project(s) ['hipblaslt/tensilelite']; no tests will be selected for them ["hipblaslt/tensilelite"] $ echo $? 0 ``` The consequence is visible in rocm-libraries. TensileLite-only PRs ran a sanity check and nothing else: | PR | files | multi-arch test components | |---|---|---| | ROCm/rocm-libraries#11518 | `tensilelite/src` + 2 new `tensilelite/tests/*.cpp` | no `Test tensilelite` | | ROCm/rocm-libraries#11896 | 7, all under `projects/hipblaslt/tensilelite/**` | sanity only | | ROCm/rocm-libraries#11877 | 6, all under `projects/hipblaslt/tensilelite/**` | sanity only | ROCm/rocm-libraries#11518 merged that way and was reverted by ROCm/rocm-libraries#12022 after failing `Test tensilelite` on gfx94X and gfx950 in this repo's CI — after merge, blocking the submodule bump in #8183. ## Technical Details One alias entry. `tensilelite` is already a synthetic node at level 3 (`test_policies.toml`), so selection also reaches `hipblaslt`, `rocblas` and `hipblas` transitively — the same set `shared/stinkytofu` already gets. After the change, on the real committed graph: ```console $ python test_tools/determine_rocm_test_dependencies.py \ --changed-projects projects/hipblaslt/tensilelite ["hipblas", "hipblaslt", "hipblasltprovider", "hipdnn-samples", "hipsolver", "hipsparse", "hipsparselt", "miopen", "miopenprovider", "rocalution", "rocblas", "rocsolver", "rocsparse", "rocwmma", "tensilelite"] ``` ## Risk Assessment Risk 2. Test-selection only; no product or device behaviour changes. It strictly widens selection for one input that currently selects nothing, so the failure mode is longer CI on TensileLite PRs, not reduced coverage. ## Related - Work tracking: ROCm/rocm-libraries#11784 - Introduced by: ROCm/rocm-libraries#11785 (rocm-libraries half of the nested-subtree fix) - Companion PRs: #7998 (TensileLite as a synthetic node), #7999 (longest-prefix match) - Fallout: ROCm/rocm-libraries#11518, reverted by ROCm/rocm-libraries#12022, blocking #8183 ## Testing Summary - Unit: new regression test in `determine_rocm_test_dependencies_test.py`, plus the existing suite. - Manual: CLI run against the real committed graph, before and after. ## Testing Checklist - [x] New regression test — `python -m unittest test_tools.tests.determine_rocm_test_dependencies_test -k nested_tensilelite` — Status: Passed - [x] Verified it fails without the fix (stashed the alias, test fails) — Status: Passed - [x] Full selector suite — `python -m unittest test_tools.tests.determine_rocm_test_dependencies_test` — 70 tests — Status: Passed - [ ] PR CI — Status: Pending The new test declares `[synthetic.tensilelite]` in its fixture, mirroring the real `test_policies.toml`, so it exercises the walk past the alias rather than stubbing it. Clean `stderr` is asserted deliberately: the regression was a warning plus an empty selection, not a non-zero exit. ## Device / Architecture Coverage None. Change-detection logic only; no compiled code and no device behaviour. PR CI is sufficient. ## Adjacent Tests Considered `shared/stinkytofu`, `shared/origami`, `shared/mxdatagenerator` and `shared/tensile` all resolve through the same alias table and are covered by `test_shared_blas_prefixes_mapped`; all still pass. ## Follow-ups not in this PR 1. **`projects/hipblaslt` still does not select `tensilelite`.** A non-TensileLite hipBLASLt change selects `[hipblas, hipblaslt, hipblasltprovider, hipsparselt, miopen, rocblas]`. rocm-libraries' `therock_matrix.py` maps `projects/hipblaslt` → `blas` → `[hipblaslt, rocblas, hipblas, tensilelite]`. Closing that gap means adding `tensilelite` as a consumer of `hipblaslt` in the consumer graph, which is a larger change than an alias and wants its own review. 2. **Unmapped `projects/` paths fail open.** `_EXTERNAL_ONLY_NAMESPACES` makes an unmapped `shared/` path raise, but an unmapped `projects/` path warns and exits `0`, yielding a green run with no tests. Extending the fail-closed behaviour would have surfaced this on the day ROCm/rocm-libraries#11785 merged rather than via a revert a week later. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Problem
TensileLite-only changes in this repo don't correctly trigger hipblaslt retesting in Multi-Arch CI:
projects/hipblaslt/tensilelitecollapses intoprojects/hipblasltin change detection (2-segment path truncation), and TheRock's consumer graph has no node for TensileLite anyway (it has no CMake target). This PR is the rocm-libraries half of the fix; see #11784 for the full tracking issue and the two companion TheRock PRs (ROCm/TheRock#7998, ROCm/TheRock#7999).Changes
repos-config.json: registerprojects/hipblaslt/tensileliteas a nested subtree entry, withauto_subtree_pull/auto_subtree_pushbothfalse-- it isn't independently synced (TensileLite lives inside hipBLASLt's own tree; hipblaslt's existing entry already covers real subtree sync). This entry exists purely so change detection has a target to match against.pr_detect_changed_subtrees.py: same longest-prefix-match fix as the companion TheRock PR, so a file underprojects/hipblaslt/tensilelite/...resolves to that nested subtree instead of collapsing intoprojects/hipblaslt.therock_matrix.py: the new, more specificprojects/hipblaslt/tensileliteselector would otherwise silently fall out of three hardcoded lookups in the legacy single-arch CI path (subtree_to_project_map,ROCJITSU_RACE_CHECK_SUBTREES,SUBTREE_EXTRA_MATRIX_PROJECTS), all keyed on the literalprojects/hipblasltstring. Added matching entries forprojects/hipblaslt/tensileliteto each, so legacy behavior is exactly preserved (same "blas" bucket, same rocjitsu race-check selection, same sparselt activation as a hipblaslt-proper change) -- I caught this by running the full.github/scripts/testssuite, not just the directly-touched test file; two legacy tests broke without this.Testing
python -m pytest .github/scripts/tests-- 153 passed, 8 xfailed (pre-existing, unrelated)pre-commit run(black) on all touched filesrepos-config.jsonentry loads cleanly through the existing PydanticRepoConfigmodelNotes
auto_subtree_pull/auto_subtree_pushare bothfalsefor the new entry, so it's excluded from every actual subtree pull/push automation (pr-merge-sync-patches.ymlpasses--require-auto-push, which filters it out before matching). Its only purpose is more precise change-detection.Tracked in #11784, alongside the two companion TheRock PRs (ROCm/TheRock#7998, ROCm/TheRock#7999).