Refactor new_amdgpu_family_matrix to use data classes instead of dictionary

[HereThereBeDragons]

Replaces the nested dictionary format in new_amdgpu_family_matrix.py with two new files using data classes as suggested in #3452:

• new_amdgpu_family_matrix_types.py — dataclass schema definitions
• new_amdgpu_family_matrix_data.py — matrix data (arch, predefined arch groups, build variants)

This will allow for easier maintenance of the entire matrix: Parameters must now be defined and have the description right next to it (instead of being spread throughout the file) and only non-default values need to be provided for each arch. In addition, helper functions provided by the matrix classes will simplify the new matrix generator logic (replacement of configure_ci.py).

Changes:

• Typed fields with defaults — only non-default values need to be specified per entry
• Arch entries got flatten to sepearate _GFX* MatrixEntry definitions
• Auto-discovery of _GFX* entries — adding a new GPU requires only a new variable, no registration step
• Added unittests

Test Config:

• run_tests auto-determined based on if a runner is set or not; set explicitly to False to reserve a runner without enabling tests
• GpuRunners.extra — catch-all dict[str, str] for non-standard or temporary runners (e.g. test-sandbox, oem) without schema changes
• New param to define which tests are to run: TestConfig.test_scope — typed Literal["all", "smoke", "full"]

Introducing new API for accessing the amdgpu matrix:

• Case-insensitive key lookup — get_entry("GFX950") and get_entry("gfx950") are equivalent
• Family-level default resolution — get_entry("gfx950") resolves to gfx950-dcgpu via is_family_default
• get_entries_for_groups — resolves to a predefined group list (e.g. amdgpu_presubmit) to the corresponding MatrixEntry objects in definition order (--> will allow to remove this step from the generator logic)
  • to_dict(platform=None) — serializes full matrix or a single platform config with amdgpu_family key included

Includes changes of:

• gfx90X split — gfx906, gfx908, gfx90a registered as individual entries ( Split gfx906/gfx908/gfx90a into separate families #2869 )
• removed push_on_success as not used anymore

[HereThereBeDragons]

what is missing (as not yet merged): #3564

[ScottTodd]

There are some useful improvements to code structure in here but I think some of the details around targets/families aren't yet in alignment with what the CI/CD systems need at their core. I also think that having a separate set of new_* scripts is going to keep biting us. Need to complete the migration so we only have one set of files serving the same purpose at a time.

[ScottTodd]

This is not an exhaustive list. See https://github.com/ROCm/TheRock/blob/main/cmake/therock_amdgpu_targets.cmake. We also have igpu for example.

acronym	meaning
dgpu	discrete gpu
dcgpu	datacenter gpu
igpu	integrated gpu

The concept of GPU families is also going to be less consequential when we've switched fully over to the multi-arch pipelines, so let's be careful about what abstractions we add here and if they will continue to make sense.

Looking through that lens and stepping back a bit, I don't understand the distinction this is making between a "family" and a "scope". That CMake file is the source of truth, and it defines a family as the full string (e.g. gfx94X-dcgpu), NOT just a particular prefix (e.g. gfx94X).

• For (non-multi-arch) CI/CD, we choose a list of target families to build for, as each family can then share device-agnostic build stages and amortize binary size overhead.
• For multi-arch CI/CD we're going to choose a list of targets to build and those MAY be grouped by family, but they don't need to be as the build will be efficiently parallelized even if they aren't. Similarly the packages will share the same device-agnostic files no matter how the GPU targets/families are split.
• For testing, we should make it clear what the mappings are for "gpu target" to "runner label". We could try to make those mappings all be 1:1 ("gfx1151" -> "linux-gfx1151-gpu-rocm") instead of being 1:N ("gfx110X-all" -> ["linux-gfx1100-gpu-rocm", "linux-gfx1101-gpu-rocm"]).

I don't think we should focus too much on "dcgpu", "dgpu", "all", etc. at this level of the infra.

[HereThereBeDragons]

i know this is not an exhaustive list, i guess adding ... will help to avoid this confusion and expanding the frozenset of _GENERIC_SCOPES (but right now we have no entry for igpu.. :) )

the idea for family and scope is similar to family and target - just that scope is agnostic if it defines a single gpu arch or a family (to avoid ambiguities i named it "scope"). this is mainly done due to the CI scripts currently accepting inputs like "gfx94X" and expanding it to "gfx94X-dcgpu". However, we have some families that have multiple igpu, dgpu etc. - so it is not clear to what it should be expanded to - or to be exact: right now this is not possible in amdgpu_family_matrix to have multiple entries for the same family. E.g. having entries for both gfx110X-all and gfx110X-igpu with different runners would not be possible.

As such i added the is_family_default, so if we say: gfx110X-all == is_family_default,you get the following behavior within the CI:

• gfx110X --> will auto-expand to gfx110X-all
• gfx110X-all will match gfx110X-all
• gfx110X-igpu will match with entry _GFX110X_IGPU if available, otherwise fail

-igpu and -all can have different settings, runners, etc.

If we put it in the right order in the file, we could probably even deep-copy the settings and just adjust what we need to.

[ScottTodd]

I'm still concerned here. This looks like a lot of boilerplate that isn't going to be useful in the multi-arch CI/CD world (cutover is imminent - please no new feature work on the older CI).

Target families and scopes, at least in their current form, are a concept that was engineered for the non-multi-arch CI.

• We need a direct mapping from "gpu architecture" to "runner label".
• We need a direct mapping from "gpu architecture" to "rocm build status", "rocm test status", "pytorch build status", etc.

We could list targets directly and construct families on top of them. I think this is inverted:

    GFX1151 = MatrixEntry(
        family="gfx115X",
        scope="gfx1151",
        linux=PlatformConfig(

I think I'd rather have something more like:

GFX1151 = GPU_ARCHITECTURE(...)
...

FAMILY_GFX1151 = [GFX1151]
FAMILY_GFX110X_ALL = [GFX1100, GFX1101, GFX1102]

@marbre may need your eyes here too.

[marbre]

Discussed this offline wit Laura, she will provide an update on what we concluded on.

[HereThereBeDragons]

we decided on the following considering that multi arch will most likely in the future only be operating on specific targets/archs:

• remove scope, and remove having "-dcgpu" or any other family specification as its own entry (as it is currently in amdgpu_family_matrix). instead have only entries for concrete archs (=target).
• for each target, auto-populate its families based on cmake/therock_amdgpu_targets.cmake
• have getters for: get "default" target for when requesting a family (for backward compatibility). is currently based on which gpu runners we have. e.g. gfx120X-all --> returns entry gfx1201.
• have getters for: "give me all entries for a family" so in the future we could replace "fetch-gfx-targets": "gfx120X-all" --> ["gfx1200", "gfx1201"]
• have a lookup() that tells you how an entry is resolved: gfx942 -> "target", gfx94X-dcgpu -> "family", gfx94X -> "family_prefix" (see question below)
• when requesting a list of gpu entries, deduplicate by default: get_entries_for_groups(["gfx942", "gfx94X-dcgpu"]) --> just returns a single entry "gfx942"

NOTE: families are only considered for all the entries which have a MatrixEntry in the file. e.g. we do not consider any "igpu" as we dont have any entries for them. so when requesting a family "-all" those are omitted as no entry exists for them.

open questions:

• right now we have partial matching of families. however it is difficult to decide which family it should be expanded to. e.g. "gfx120X" --> returns entry "gfx1201". but if we want to set amdgpu_family: in the workflow, we kinda need to expand it to "gfx120X-all". so how do we do it? or do we just remove this functionality?

see claude reply:

❯ i wonder if we should just remove the gfx94X matching. as we need to know exactly if gfx94X-all or gfx94X-dcgpu to know what we are building                                                 

● That's a valid point. The prefix match hides an important distinction — gfx94X-dcgpu builds only discrete GPU targets, while gfx94X-all would include everything in the family. Silently
  picking whichever family happens to match first is ambiguous and could cause wrong builds.

  The arguments for removing it:
  - Forces callers to be explicit about what they're building
  - Removes ambiguity when a family has multiple scopes (dcgpu vs all)
  - The only convenience lost is saving 6 characters (-dcgpu / -all)

  The argument for keeping it: gfx94X is a natural shorthand people might type, and it's better to resolve it than error. But that's a weak argument — failing with "did you mean gfx94X-dcgpu
  or gfx94X-all?" is more helpful than silently picking one.

future questions:

• what happens if we request a family but have multiple runners? do we return a list of runners? do we keep the "just return the default_family_entry"? something else? or is this just not a scenario that could be happening?
• i think we can auto-populate fetch-gfx-targets but it would be good to know what the exact requirements are, as for some it returns multiple targets and for others just a single one eventhough we have more entries for the same family

[ScottTodd]

As I understand, postsubmit workflows run the union of the "presubmit" and "postsubmit" sets. Maybe we should either:

1. Document that here in comments
2. Actually build the lists that way? A few options:

   # Option A: construct sets separately
   amdgpu_presubmit_set = set(["gfx94X-dcgpu", "gfx110X-all", "gfx1151", "gfx120X-all"])
   amdgpu_postsubmit_set = amdgpu_presubmit_set | set(["gfx950-dcgpu"])
   amdgpu_nightly_set = amdgpu_postsubmit_set | set([
           "gfx906",
           "gfx908",
           "gfx90a",
           "gfx101X-dgpu",
           "gfx103X-dgpu",
           "gfx1150",
           "gfx1152",
           "gfx1153",
       ])
   amdgpu_family_predefined_sets = {
     "amdgpu_presubmit": amdgpu_presubmit_set,
     "amdgpu_postsubmit": amdgpu_postsubmit_set,
     "amdgpu_nightly": amdgpu_nightly_set,
   }

   # Option B: construct a single list and then parse out the individual sets
   from enum import Enum
   class TriggerFrequency(Enum):
       PRESUBMIT = 1
       POSTSUBMIT = 2
       NIGHTLY = 3
   
   amdgpu_family_groups = {
     # Presubmit
     "gfx94X-dcgpu": TriggerFrequency.PRESUBMIT,
     "gfx110X-all": TriggerFrequency.PRESUBMIT,
     ...
     # Postsubmit
     "gfx950-dcgpu": TriggerFrequency.POSTSUBMIT,
     ...
   }
   # Then construct sub-lists from that full list?

I'm basically looking for a design here that makes it

1. easy to see which GPU targets/families are build/tested by default for each event
2. easy to move build/test frequency up or down
3. hard to make configuration mistakes

[HereThereBeDragons]

option 1 seems for me way easier to understand and extend. for the future we might want to have additional predefined groups for weekly runs etc.

[ScottTodd]

Oh, this is an interesting way to collect entries into a single list without needing to explicitly add them. Could put them in a class scope to avoid needing to use globals() though?

[HereThereBeDragons]

yeah i wanted to make it less error prone when editing. adding a new arch is already annoying enough considering over how many lines a single entry spans - and then keeping manually track of which are available, changed naming or whatever is just horrible

[HereThereBeDragons]

yes, i know the new_* is biting us but I am missing some extra hands to get this more quickly done :(

[geomin12]

much cleaner i like it!!! just one comment but lgtm

[geomin12]

there are more of these args coming, it may be good to add that extra dict here as well, as we add/remove optional args

[ScottTodd]

#3992 also added quick as a test scope/type

[geomin12]

yes, i know the new_* is biting us but I am missing some extra hands to get this more quickly done :(

I think it is fine for the time being!

[erman-gurses]

Added some comments

[erman-gurses]

get_entries_for_groups() currently skips unknown keys silently. For CI configuration, that feels risky because a typo in a predefined group could quietly reduce build/test coverage instead of failing fast. Typo like gfx94X-dcgup would just disappear from the result

I think this should raise on unknown keys, or at least have a strict mode used by tests / workflow generation.

[HereThereBeDragons]

i will add that not only the matching entries are returned but also unmatched keys.

[HereThereBeDragons]

please also note that while predefined groups are being used, in theory any list of strings can be used as group_keys

[erman-gurses]

I see your point. How about something like:
def get_entries_for_groups(group_keys, strict=False):

When strict=False, unknown keys can be skipped (for cases like experimental or user-provided keys).
When strict=True, the function could raise if any key is not recognized.

That way the helper stays flexible for general use, but CI/workflow generation can use strict=True to avoid silently dropping entries due to typos.

[HereThereBeDragons]

@ScottTodd any opinion on it?
i would argue that giving back a tuple of match entries, and unmatched keys is enough and any script on top can have the logic to fail if needed.

[erman-gurses]

Would be good If we get @ScottTodd's opinion. I see that your approach gives more flexibility on the caller side.

[erman-gurses]

Since get_entries_for_groups() is used on predefined CI groups, maybe we can add a negative test for an unknown key here. Otherwise a typo in a group can silently pass through without coverage loss being obvious.

For example, a test that verifies unknown keys either raise or are handled explicitly would make this safer.

[erman-gurses]

Do we use this method anywhere? I may have missed it, but I didn’t find a usage in this PR or in the added tests.

[HereThereBeDragons]

we will be using it to create the dictionary that we use in the workflows later (created by the refactored configure_ci.py which will be the next PR step).
can see if i can add a test

[HereThereBeDragons]

will add some testing. but not sure how useful it really is

[erman-gurses]

It was just a suggestion - maybe we can add after its usage is active.
As far as I understand, we can do something like this in the future for regression testing.

assert result["gfx94X"]["build"]["amdgpu_family"] == "gfx94X"

[ScottTodd]

Sorry, wish I could have given more of this feedback sooner, but I'm still not really seeing the dots fully connecting here. The "GPU family" code is adding significant complexity. I think for multi-arch we need to trim that out and make it less foundational to the CI build/test configuration code paths.

I think my comment on a flatter list of GpuRunners is trending in a cleaner direction.

[ScottTodd]

This could also raise KeyError instead of returning None. WDYT?

[HereThereBeDragons]

i would keep it with None as this is kinda the expected behavior for python

get(key, default=None, /)
Return the value for key if key is in the dictionary, else default. If default is not given, it defaults to None, so that this method never raises a KeyError.

https://docs.python.org/3/library/stdtypes.html#dict.get

[ScottTodd]

Is "unmatched keys" useful to track like this, or could we just raise KeyError instead? Is there a use case for passing in groups that aren't in the code and getting this sort of detailed reporting on a successful (non-raising) execution flow?

[HereThereBeDragons]

considering that get() above returns None i sadly think the current approach is the more uniform approach than raising here keyerror.

discussion with claude:
Python style:

• GroupLookupResult as a @DataClass instead of a tuple directly follows the style guide ("no tuples for structured data")
• The None-returning get_entry mirrors dict.get() — consistent with standard Python lookup conventions
• Returning partial results with an explicit miss report is not "silently continuing on errors" — it surfaces the problem. The style guide's fail-fast rule targets dropping errors, not delegating them to the
  caller

Architecture:

• get_entries_for_groups is a bulk filter operation, not a dict lookup. Bulk operations in Python conventionally report partial results rather than raising on the first miss (e.g., how `pathlib.Path.glob() just
  returns what matches, or how importlib handles missing submodules)
• The caller-decides-strictness pattern is standard for library code — the function is a primitive, not a policy enforcer

The one thing that would violate the fail-fast principle is if unmatched_keys were silently ignored downstream. But that's a caller problem, not an API problem. The API does its job by surfacing them.

[ScottTodd]

• Returning partial results with an explicit miss report is not "silently continuing on errors" — it surfaces the problem. The style guide's fail-fast rule targets dropping errors, not delegating them to the
  caller

Can you give an example of where the caller would provide a list of groups that contains invalid entries? As much as possible, I'd like for the CI system (anything in build_tools/github_actions/) to be correct by construction and not need to patch around issues that it causes on its own. We can add unit tests to enforce only valid GPU targets/groups/families in any configurations (e.g. "run these tests nightly", "run these tests on every PR"). When there is user input (e.g. workflow dispatch text fields), we can validate those inputs during parsing.

I'm not sure if we even need this get_entries_for_groups() function that deduplicates and tracks unmatched. How do you see get_entries_for_groups() and even get_entry() being used? I suspect we could get away with just get_entry() and have that raise KeyError instead of returning None too.

[HereThereBeDragons]

get_entries_for_group() is the entry point for the workflow files.

the idea is that you take the comma-separate inputs.amdgpu_families, the configure_ci.py transforms it in a list and then you call get_entries_for_group() and get back all matching MatrixEntries. Later you can call MatrixEntry.to_dict(platform=linux) to get the dictionary of that particular arch for the linux platform (or None)

So I thought having an easy way of reporting back on non-matching archs might be nice :)

[ScottTodd]

This test case is very similar to one above:

    def test_family_prefix_resolves_for_gfx110X(self):
        entry = amdgpu_family_info_matrix_all.get_entry("gfx110X")
        self.assertIsNotNone(entry)
        self.assertEqual(entry.key, "gfx1101")

Maybe merge the test cases into a single test? Or put them next to each other?

I'm not sure how useful it is to have both tests. Maybe a single "prefix" test and then a negative test showing that something like gfx110X-unknowngroup raises an exception?

[ScottTodd]

Are these tests useful? They look like change detector tests just checking the default field values which are trivially checkable by reading the builder functions.

[ScottTodd]

The operator overloading here doesn't feel obvious to me. Could pick a function name like has_any_runner

def has_any_runner(self) -> bool:
  return any([self.test, self.test_multi_gpu, self.benchmark, self.extra])

[HereThereBeDragons]

you mean how run_tests is set?
that it is true by default if there are any runners and not overwritten to false?

for me this feels like that makes life quite simple once you get use to it?
i mean we can pack it in an extra function but then you in the ci you need to check 2x - or you just want to rename the funciton?

[ScottTodd]

I'm referring to the function name (overloading __bool__).

Here are some other style guide references:

• https://google.github.io/styleguide/pyguide.html#213-properties

  Properties are allowed, but, like operator overloading, should only be used when necessary and match the expectations of typical attribute access; follow the getters and setters rules otherwise.

• https://google.github.io/styleguide/cppguide.html#Operator_Overloading

  Pros:
  Operator overloading can make code more concise and intuitive by enabling user-defined types to behave the same as built-in types. Overloaded operators are the idiomatic names for certain operations (e.g., ==, <, =, and <<), and adhering to those conventions can make user-defined types more readable and enable them to interoperate with libraries that expect those names.

  Cons:

  • Providing a correct, consistent, and unsurprising set of operator overloads requires some care, and failure to do so can lead to confusion and bugs.

Where this is used, I think this reads more clearly:

    def __post_init__(self):
-       # set run_tests to True if any runners are specified,
-       # False otherwise, if not explicitly set
+       # Default to running tests if any runner is specified.
        if self.run_tests is None:
-           self.run_tests = bool(self.runs_on)
+           self.run_tests = self.runs_on.has_any_runner()

[ScottTodd]

#3992 also added quick as a test scope/type

[ScottTodd]

What is a "family default"? I'm struggling to see what problem that is solving.

I think this gets back to my original first-pass review comments. This is adding indirection that we shouldn't need, and it's going to make changing this code harder.

The dataflow through

• Configure CI
• List of GPU targets
• List of GPU runner labels

should be pretty straight-line, and this further encodes a hierarchy of "familes" with "defaults". If we add gfx1100, gfx1101, and gfx1102 runners for example, this code would keep resolving the "default" for "gfx110X-all" as "gfx1101" when what we really should be doing is passing that full list of gfx1100, gfx1101, gfx1102 directly through the pipeline.

I don't think we should design around internal code checking for gfx110X-all (for example), at all. We can design around workflow inputs that use such convenience families at the API boundary but the CI code should directly use what it intends, without needing all that extra machinery for prefixes / grouping / defaults.

When we build generic targets (https://llvm.org/docs/AMDGPUUsage.html, e.g. gfx11-generic), then I think a "default" of some sort may make sense, but I'm not seeing the value right now. We'll actually want a different one-to-many relationship in that case, like

GFX11_GENERIC = MatrixEntry(
      target="gfx11-generic",
      linux=PlatformConfig(
          test=TestConfig(
              runs_on=[
                  GpuRunners(test="linux-gfx1100-gpu-rocm", gfx_target="gfx1100"),
                  GpuRunners(test="linux-gfx1151-gpu-rocm", gfx_target="gfx1151"),
              ],
          ),
      ),
  )

Here's a write-up from my review agent after some discussion: https://github.com/ScottTodd/claude-rocm-workspace/blob/main/reviews/pr_3653.md#%EF%B8%8F-important-family-resolution-layer-should-be-removed

[HereThereBeDragons]

we can remove it.
i added it to have some mapping from "someone puts manually gfx110X as gpu in the workflow" to "requesting gfx110X means calling gfx110X-all".

[ScottTodd]

Yeah convenience mappings like "gfx110X input" --> "build [gfx1100, gfx1101, gfx1102, gfx1103] and test on any of those that have runners" are fine. Looking further forward, if we add a web UI to trigger CI/CD jobs we can add UI options for that like:

then, the code at the CI layer could just trust its input to already be sanitized/expanded.

[HereThereBeDragons]

ok then we leave it. i will see if i can clarify this a bit better in the comments.

[ScottTodd]

Here's an idea: what if we pulled the GpuRunners out into a single list, as an inventory of what test machines we have?

  runners = [
      GpuRunner(platform="linux",   target="gfx942",  label="linux-mi325-1gpu-ossci-rocm-frac"),
      GpuRunner(platform="linux",   target="gfx942",  label="linux-mi325-8gpu-ossci-rocm", role="test-multi-gpu"),
      GpuRunner(platform="linux",   target="gfx942",  label="linux-mi325-8gpu-ossci-rocm", role="benchmark"),
      GpuRunner(platform="linux",   target="gfx90a",  label="linux-gfx90a-gpu-rocm"),
      GpuRunner(platform="linux",   target="gfx1030", label="linux-gfx1030-gpu-rocm"),
      GpuRunner(platform="linux",   target="gfx1151", label="linux-gfx1151-gpu-rocm"),
      GpuRunner(platform="linux",   target="gfx1151", label="linux-strix-halo-gpu-rocm-oem", role="oem"),
      GpuRunner(platform="windows", target="gfx1101", label="windows-gfx110X-gpu-rocm"),
      GpuRunner(platform="windows", target="gfx1151", label="windows-gfx1151-gpu-rocm"),
      GpuRunner(platform="windows", target="gfx1151", label="windows-gfx1151-gpu-rocm", role="benchmark"),
      # ...
  ]

(we could also have separate lists for Windows and Linux, to make the code more dense)

Code would query that list of runners when deciding how to test. If we want to take a runner offline we can comment out that row. If we want to pull that list from a dynamic location we don't need to update the entire build configuration (asan/release/etc.) too.

Here's how some of the other code using this could look:

# Matrix entries — per-target-per-platform policy
 entries = [
      MatrixEntry(target="gfx942",  platform="linux"),
      MatrixEntry(target="gfx1151", platform="linux",   sanity_check_only=True),
      MatrixEntry(target="gfx1151", platform="windows", test_scope="full"),
      MatrixEntry(target="gfx1152", platform="linux",   expect_build_failure=True),
      MatrixEntry(target="gfx1152", platform="windows", expect_build_failure=True),
      # ...
  ]

  # Group configs — what CI does per trigger
  presubmit = GroupConfig(
      build_families=["gfx94X-dcgpu", "gfx110X-all", "gfx115X-all", "gfx120X-all"],
      test_targets=["gfx942", "gfx1101", "gfx1151", "gfx1201"],
      build_variants=["release"],
  )

Here's a more concrete proposal: https://github.com/ScottTodd/claude-rocm-workspace/blob/main/reviews/pr_3653.md#proposed-alternative-three-flat-tables

[HereThereBeDragons]

i am all in in putting the gpurunners in a separate list. (i would also hope in the future we have a database to populate it. then we also dont have problems with outdated runners names)

however i think the rest of the proposal of claude is a downgrade and i would rather not.

[ScottTodd]

I'm proposing a similarly structured list in some new code here: #4386

@dataclass(frozen=True)
class S3BucketConfig:
    """Metadata for a single bucket in S3"""

    name: str
    """S3 bucket name (e.g. 'therock-ci-artifacts')"""

    ...

s3_bucket_configs = [
    # CI (self-hosted runners include credentials for therock-ci-artifacts-external)
    S3BucketConfig("therock-ci-artifacts", iam_role="therock-ci"),
    S3BucketConfig("therock-ci-artifacts-external", iam_role=None),
    # Release type "dev"
    S3BucketConfig("therock-dev-artifacts", iam_role="therock-dev"),
    S3BucketConfig("therock-dev-packages", iam_role="therock-dev"),
    S3BucketConfig("therock-dev-python", iam_role="therock-dev"),
    S3BucketConfig("therock-dev-tarball", iam_role="therock-dev"),
    ...
]

_BUCKET_CONFIGS_BY_NAME = {c.name: c for c in s3_bucket_configs}

...

def get_artifacts_bucket_config(...) -> S3BucketConfig:
    ...
    bucket_name = "therock-ci-artifacts"
    return _BUCKET_CONFIGS_BY_NAME[bucket_name]

[HereThereBeDragons]

sounds good. i will have a look how to best adapt this.

[HereThereBeDragons]

New AMD GPU Family Matrix — what's new and how to use it

TL;DR: amdgpu_family_matrix.py (the big dict-of-dicts) is being replaced by three dataclass-based modules. Same data, but typed, validated, and with a real lookup API. Old file stays in place during transition — keep both in sync.

The matrix now also takes a lot of heavy lifting off configure_ci.py — build-variant scoping (dropping platforms that don't support a variant, trimming build_variants to the requested one), build-runner re-binding for non-release variants, GPU runner inventory selection, and run_tests inference all happen inside get_entry() now, so callers just consume the result.
Manual intervention is only needed for special runners like "oem" that will be delivered in the ["extra"] field and needs to be set for test/test_multi_gpu/etc.

Files

File	Purpose
new_amdgpu_family_matrix_types.py	Dataclass schema (no data)
new_amdgpu_family_matrix_data.py	The actual matrix entries — this is where you add/edit GPUs
new_amdgpu_family_matrix_runners.py	Build/GPU runner inventories with weighted selection — this is where you add/edit Runners

How to look up an entry

from new_amdgpu_family_matrix_data import amdgpu_family_info_matrix_all as matrix

# Exact target name
entry = matrix.get_entry("gfx942")

# Or family name — resolves to the is_family_default entry
entry = matrix.get_entry("gfx94X-dcgpu")

# Build variant scoping (default is "release")
asan = matrix.get_entry("gfx942", build_variant="asan")  # None if unsupported

# Multiple keys at once
result = matrix.get_entries_for_groups(["gfx942", "gfx1151"])
result.entries          # matched MatrixEntry list
result.unmatched_keys   # keys with no match (unknown or unsupported variant)

get_entry() always returns a deep copy scoped to the requested variant — mutate freely, no leakage into the shared matrix.

Important behavior changes vs. the old dict

1. Family membership is auto-populated from cmake/therock_amdgpu_targets.cmake — no longer hand-listed per entry. CMake is the source of truth.
2. Runners are no longer hardcoded per entry. BuildConfig.runs_on and TestConfig.runs_on are filled from the inventory in _runners.py (with weighted selection across pools). Override per-entry only when you need to pin a specific runner. (already includes [ci] Enabling aws-linux-scale-rocm to 10%  #4899)
3. run_tests is inferred from whether any GPU runner is set. Set it explicitly to True/False only to override.
4. Build variants (release / asan / tsan) are first-class. Looking up a non-release variant drops platforms that don't list it and re-binds the build runner to the matching pool.
5. is_family_default marks the entry returned for family-name lookups (e.g. gfx94X-dcgpu → which specific gfx target). Validated at construction — at most one default per gfx-prefixed family.
6. Test scope is now a Literal["quick", "comprehensive", "full"] — typo-safe.
7. bypass_tests_for_unscheduled lets you skip tests on PR/push but still run on nightly schedule. (renamed from nightly_check_only_for_family )
8. Add a MatrixEntry to _MatrixEntries in _data.py — that's it. Family lists are derived from CMake; runners come from the inventory. Then mirror the change in the legacy amdgpu_family_matrix.py until that file is removed.

Adding a new field

• Add to the right dataclass in _types.py with a default
• If it's derived, compute it in __post_init__
• Add to to_dict() if consumers need it in the serialized output

Notes

• not yet included: [ci] Adding host-asan for postsubmit #4603

[HereThereBeDragons]

btw @geomin12 volunteered to help port the multi arch configure ci to use this new version :)

and i think this is a good point to kill off/stop supporting the old configure_ci.py

[HereThereBeDragons]

#4613 for tracking

[jayhawk-commits]

Still need some time to digest the design in this latest state,, but Codex did find a difference in behaviour from the current matrix.

In amdgpu_family_matrix.py, Linux tsan is defined without an expect_failure override, so current consumers treat TSAN as blocking. With this replacement data, BuildVariantInfo.to_dict() will serialize expect_failure=True, and a future migration would turn TSAN into a continue-on-error job unless that policy change is deliberate.

[ScottTodd]

In amdgpu_family_matrix.py, Linux tsan is defined without an expect_failure override, so current consumers treat TSAN as blocking. With this replacement data, BuildVariantInfo.to_dict() will serialize expect_failure=True, and a future migration would turn TSAN into a continue-on-error job unless that policy change is deliberate.

expect_failure isn't even in use or working right now. I think we should remove it, see #4500

[geomin12]

looks good! i did a cross reference with amdgpu_family_matrix.py, and all is covered (also checked with claude). have some housekeeping comments on easy of viewing and some additional entries

[geomin12]

gfx90c was added as well!

TheRock/cmake/therock_amdgpu_targets.cmake

Line 58 in 8d042a2

therock_add_amdgpu_target(gfx90c "AMD Renoir/Lucienne/Cezanne iGPU" FAMILY igpu-all gfx90c-igpu

[geomin12]

should we perhaps set these fields in the _MatrixEntries? that way we don't need to hard-code this and can collect the matrices from _MatrixEntries?

[geomin12]

can we add some comment that is a temporary stopgap? i really don't like separate labels and having to adjust weights. i plan to work with OSSCI to update

[geomin12]

nit: can we make this new line like the bigger entries? for consistency... although i don't know if pre-commit will complain or not

[geomin12]

same for all these entries

[geomin12]

can we add over-arching comments to highlight the differences?

Suggested change

	_BuildRunnerEntry(
	platform="linux", variant="default", label="azure-linux-scale-rocm", weight=90
	),
	_BuildRunnerEntry(
	platform="linux", variant="default", label="aws-linux-scale-rocm", weight=10
	),
	_BuildRunnerEntry(
	platform="linux",
	variant="sanitizer",
	label="azure-linux-scale-rocm-heavy-ramdisk",
	weight=1,
	),
	_BuildRunnerEntry(
	platform="windows",
	variant="default",
	label="azure-windows-scale-rocm",
	weight=1,
	),
	# default
	_BuildRunnerEntry(
	platform="linux", variant="default", label="azure-linux-scale-rocm", weight=90
	),
	_BuildRunnerEntry(
	platform="linux", variant="default", label="aws-linux-scale-rocm", weight=10
	),
	# sanitizer
	_BuildRunnerEntry(
	platform="linux",
	variant="sanitizer",
	label="azure-linux-scale-rocm-heavy-ramdisk",
	weight=1,
	),
	_BuildRunnerEntry(
	platform="windows",
	variant="default",
	label="azure-windows-scale-rocm",
	weight=1,
	),

something like so? it was initially tough to spot

[geomin12]

oh, can we make this have new lines with each entry? to make it easier to adjust / consistency with others

[geomin12]

way cleaner, thanks :)

[geomin12]

is sandbox supposed to be in here?

[geomin12]

i know host-asan will be added soon with #4603

[ScottTodd]

Marking as "reviewed" to drop from my review queue.

I still think we should remove the concept of gpu "families" from this part of the CI system entirely. This PR is probably too big and complex at this point - it would be faster to start fresh on a new branch/PR.