From 4eee1c9b77ca5e1ed34a84155f65413965a5c74d Mon Sep 17 00:00:00 2001 From: Tony Davis Date: Fri, 15 May 2026 12:51:17 -0500 Subject: [PATCH 1/6] [CI] Run hipSPARSELt when hipBLASLt subtree changes 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 --- .github/scripts/tests/therock_matrix_test.py | 16 +++++++++++++++- .github/scripts/therock_matrix.py | 11 +++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/.github/scripts/tests/therock_matrix_test.py b/.github/scripts/tests/therock_matrix_test.py index 223c3ebd970..cdb5895de1c 100644 --- a/.github/scripts/tests/therock_matrix_test.py +++ b/.github/scripts/tests/therock_matrix_test.py @@ -1,4 +1,5 @@ from pathlib import Path +import importlib import os import sys import unittest @@ -8,11 +9,20 @@ class TheRockMatrixTest(unittest.TestCase): + def setUp(self): + # collect_projects_to_run mutates module-level project_map; reset between tests. + importlib.reload(therock_matrix) + def test_collect_projects_to_run_without_additional_option(self): subtrees = ["projects/hipblaslt"] project_to_run = therock_matrix.collect_projects_to_run(subtrees) self.assertEqual(len(project_to_run), 1) + blas_entry = project_to_run[0] + self.assertIn( + "hipsparselt", + blas_entry["projects_to_test"].split(","), + ) def test_collect_projects_to_run(self): subtrees = ["projects/rocsparse", "projects/hipblaslt"] @@ -36,7 +46,11 @@ def test_collect_projects_to_run_dependency_graph_diff_projects(self): subtrees = ["projects/miopen", "projects/rocwmma"] project_to_run = therock_matrix.collect_projects_to_run(subtrees) - self.assertEqual(len(project_to_run), 2) + # rocwmma only contributes via blas under additional_options; miopen absorbs blas. + self.assertEqual(len(project_to_run), 1) + combined = project_to_run[0] + self.assertIn("rocwmma", combined["projects_to_test"].split(",")) + self.assertIn("miopen", combined["projects_to_test"].split(",")) if __name__ == "__main__": diff --git a/.github/scripts/therock_matrix.py b/.github/scripts/therock_matrix.py index 013ae6ba491..30df53d8d3a 100644 --- a/.github/scripts/therock_matrix.py +++ b/.github/scripts/therock_matrix.py @@ -152,6 +152,12 @@ "miopen": ["blas", "rand", "fusilli-provider"], } +# 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", +} + def collect_projects_to_run(subtrees): platform = os.getenv("PLATFORM") @@ -161,6 +167,11 @@ def collect_projects_to_run(subtrees): if subtree in subtree_to_project_map: projects.add(subtree_to_project_map.get(subtree)) + for subtree in subtrees: + extra_matrix = SUBTREE_EXTRA_MATRIX_PROJECTS.get(subtree) + if extra_matrix: + projects.add(extra_matrix) + for project in list(projects): # Check if an optional math component was included. if project in additional_options: From 9279955901c69220d29b74ab5c6b0b824b48b456 Mon Sep 17 00:00:00 2001 From: Tony Davis Date: Fri, 15 May 2026 12:58:49 -0500 Subject: [PATCH 2/6] [ci] Touch hipblaslt path for TheRock PR matrix verification 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 --- projects/hipblaslt/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/projects/hipblaslt/CMakeLists.txt b/projects/hipblaslt/CMakeLists.txt index a1911a51865..f4a1b5ed3a4 100644 --- a/projects/hipblaslt/CMakeLists.txt +++ b/projects/hipblaslt/CMakeLists.txt @@ -1,5 +1,6 @@ # Copyright Advanced Micro Devices, Inc., or its affiliates. # SPDX-License-Identifier: MIT +# Non-functional: touches projects/hipblaslt so TheRock CI path filter exercises hipSPARSELt (see PR). cmake_minimum_required(VERSION 3.25.2) From 180dc9f70c620cb4b6a5322b4766b8a1369a31cd Mon Sep 17 00:00:00 2001 From: Tony Davis Date: Fri, 15 May 2026 14:05:56 -0500 Subject: [PATCH 3/6] Revert "[ci] Touch hipblaslt path for TheRock PR matrix verification" This reverts commit 9279955901c69220d29b74ab5c6b0b824b48b456. --- projects/hipblaslt/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/projects/hipblaslt/CMakeLists.txt b/projects/hipblaslt/CMakeLists.txt index f4a1b5ed3a4..a1911a51865 100644 --- a/projects/hipblaslt/CMakeLists.txt +++ b/projects/hipblaslt/CMakeLists.txt @@ -1,6 +1,5 @@ # Copyright Advanced Micro Devices, Inc., or its affiliates. # SPDX-License-Identifier: MIT -# Non-functional: touches projects/hipblaslt so TheRock CI path filter exercises hipSPARSELt (see PR). cmake_minimum_required(VERSION 3.25.2) From 5992e9e565d79cadc23e1b6e985ef365d448e288 Mon Sep 17 00:00:00 2001 From: Tony Davis Date: Mon, 18 May 2026 15:52:24 -0500 Subject: [PATCH 4/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .github/scripts/therock_matrix.py | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/scripts/therock_matrix.py b/.github/scripts/therock_matrix.py index 30df53d8d3a..7054774b147 100644 --- a/.github/scripts/therock_matrix.py +++ b/.github/scripts/therock_matrix.py @@ -167,7 +167,6 @@ def collect_projects_to_run(subtrees): if subtree in subtree_to_project_map: projects.add(subtree_to_project_map.get(subtree)) - for subtree in subtrees: extra_matrix = SUBTREE_EXTRA_MATRIX_PROJECTS.get(subtree) if extra_matrix: projects.add(extra_matrix) From 9c809456faebb8e16bfd7e3a6afda17ed9bd2440 Mon Sep 17 00:00:00 2001 From: Tony Davis Date: Thu, 28 May 2026 12:22:28 -0500 Subject: [PATCH 5/6] Stop mutating module-level state in collect_projects_to_run David flagged that collect_projects_to_run() extends lists, replaces values, and deletes keys directly on the module-level `project_map` and `additional_options` dictionaries. That worked for the GitHub Action's single invocation but broke any second call in the same process - including the unit tests, which were forced to importlib.reload(therock_matrix) between cases to mask the leak. Take per-call deep copies of both dicts at the top of the function and operate on those locally. Drop the importlib.reload workaround in the tests and add a regression test that snapshots the module dicts, exercises representative subtree combinations, and asserts the originals are untouched. Co-authored-by: Cursor --- .github/scripts/tests/therock_matrix_test.py | 30 +++++++++++++--- .github/scripts/therock_matrix.py | 38 ++++++++++++-------- 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/.github/scripts/tests/therock_matrix_test.py b/.github/scripts/tests/therock_matrix_test.py index cdb5895de1c..cc5c8c43269 100644 --- a/.github/scripts/tests/therock_matrix_test.py +++ b/.github/scripts/tests/therock_matrix_test.py @@ -1,5 +1,5 @@ +import copy from pathlib import Path -import importlib import os import sys import unittest @@ -9,10 +9,6 @@ class TheRockMatrixTest(unittest.TestCase): - def setUp(self): - # collect_projects_to_run mutates module-level project_map; reset between tests. - importlib.reload(therock_matrix) - def test_collect_projects_to_run_without_additional_option(self): subtrees = ["projects/hipblaslt"] @@ -52,6 +48,30 @@ def test_collect_projects_to_run_dependency_graph_diff_projects(self): self.assertIn("rocwmma", combined["projects_to_test"].split(",")) self.assertIn("miopen", combined["projects_to_test"].split(",")) + def test_collect_projects_to_run_does_not_mutate_module_state(self): + # Snapshot module-level dicts, run a series of representative calls, and + # confirm the originals are untouched. This guards against the + # mutate-globals regression that previously required importlib.reload + # between tests. + project_map_before = copy.deepcopy(therock_matrix.project_map) + additional_options_before = copy.deepcopy(therock_matrix.additional_options) + + therock_matrix.collect_projects_to_run(["projects/hipblaslt"]) + therock_matrix.collect_projects_to_run( + ["projects/rocsparse", "projects/hipblaslt"] + ) + therock_matrix.collect_projects_to_run( + ["projects/miopen", "projects/hipblaslt"] + ) + therock_matrix.collect_projects_to_run( + ["projects/miopen", "projects/rocwmma"] + ) + + self.assertEqual(therock_matrix.project_map, project_map_before) + self.assertEqual( + therock_matrix.additional_options, additional_options_before + ) + if __name__ == "__main__": unittest.main() diff --git a/.github/scripts/therock_matrix.py b/.github/scripts/therock_matrix.py index 7054774b147..4769d1d4cfb 100644 --- a/.github/scripts/therock_matrix.py +++ b/.github/scripts/therock_matrix.py @@ -2,6 +2,7 @@ This dictionary is used to map specific file directory changes to the corresponding build flag and tests """ +import copy import os subtree_to_project_map = { @@ -162,6 +163,13 @@ def collect_projects_to_run(subtrees): platform = os.getenv("PLATFORM") projects = set() + # Work on per-call deep copies so module-level state stays immutable across calls. + # Without this, the function would extend lists, replace values, and delete keys + # in `project_map` / `additional_options`, breaking any second call in the same + # process (and forcing tests to `importlib.reload` between cases). + local_project_map = copy.deepcopy(project_map) + local_additional_options = copy.deepcopy(additional_options) + # collect the associated subtree to project for subtree in subtrees: if subtree in subtree_to_project_map: @@ -173,25 +181,25 @@ def collect_projects_to_run(subtrees): for project in list(projects): # Check if an optional math component was included. - if project in additional_options: - project_options_to_add = additional_options[project] + if project in local_additional_options: + project_options_to_add = local_additional_options[project] project_to_add = project_options_to_add["project_to_add"] # If `project_to_add` is in included, add options to the existing `project_map` entry if project_to_add in projects: - project_map[project_to_add]["cmake_options"].extend( + local_project_map[project_to_add]["cmake_options"].extend( project_options_to_add["cmake_options"] ) - project_map[project_to_add]["projects_to_test"].extend( + local_project_map[project_to_add]["projects_to_test"].extend( project_options_to_add["projects_to_test"] ) # If `project_to_add` is not included, only run build and tests for the optional project else: projects.add(project_to_add) - project_map[project_to_add]["cmake_options"] = project_options_to_add[ - "cmake_options" - ] - project_map[project_to_add]["projects_to_test"] = ( + local_project_map[project_to_add]["cmake_options"] = ( + project_options_to_add["cmake_options"] + ) + local_project_map[project_to_add]["projects_to_test"] = ( project_options_to_add["projects_to_test"] ) @@ -203,24 +211,24 @@ def collect_projects_to_run(subtrees): for dependency in dependency_graph[project]: # If the dependency is also included, let's combine to avoid overlap if dependency in projects: - project_map[project]["cmake_options"].extend( - project_map[dependency]["cmake_options"] + local_project_map[project]["cmake_options"].extend( + local_project_map[dependency]["cmake_options"] ) - project_map[project]["projects_to_test"].extend( - project_map[dependency]["projects_to_test"] + local_project_map[project]["projects_to_test"].extend( + local_project_map[dependency]["projects_to_test"] ) to_remove_from_project_map.append(dependency) # if dependency is included in projects and parent is found, we delete the dependency as the parent will build and test for to_remove_item in to_remove_from_project_map: projects.remove(to_remove_item) - del project_map[to_remove_item] + del local_project_map[to_remove_item] # retrieve the subtrees to checkout, cmake options to build, and projects to test project_to_run = [] for project in projects: - if project in project_map: - project_map_data = project_map.get(project) + if project in local_project_map: + project_map_data = local_project_map.get(project) # Check if platform-based additional flags are needed if ( From 2672f2e539d5e06eb6c96789f9af5f3feb81974f Mon Sep 17 00:00:00 2001 From: Tony Davis Date: Thu, 28 May 2026 12:46:35 -0500 Subject: [PATCH 6/6] Apply black formatting to therock_matrix_test Pre-commit's black hook collapsed two short multi-line calls onto single lines. Pure formatting; no behavior change. Co-authored-by: Cursor --- .github/scripts/tests/therock_matrix_test.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/scripts/tests/therock_matrix_test.py b/.github/scripts/tests/therock_matrix_test.py index cc5c8c43269..8bea16e1ff8 100644 --- a/.github/scripts/tests/therock_matrix_test.py +++ b/.github/scripts/tests/therock_matrix_test.py @@ -63,14 +63,10 @@ def test_collect_projects_to_run_does_not_mutate_module_state(self): therock_matrix.collect_projects_to_run( ["projects/miopen", "projects/hipblaslt"] ) - therock_matrix.collect_projects_to_run( - ["projects/miopen", "projects/rocwmma"] - ) + therock_matrix.collect_projects_to_run(["projects/miopen", "projects/rocwmma"]) self.assertEqual(therock_matrix.project_map, project_map_before) - self.assertEqual( - therock_matrix.additional_options, additional_options_before - ) + self.assertEqual(therock_matrix.additional_options, additional_options_before) if __name__ == "__main__":