Add wrappers to rockrel by jonatluu · Pull Request #84 · ROCm/rockrel

jonatluu · 2026-08-07T01:21:14Z

Motivation

Adding wrapper test_rocm_wheels.yml in rockrel.

Issue: #82

Technical Details

Added additional tests to .github/workflows

Test Plan

Testing in workflows

Test Result

PASS multi arch release Linux Trigger Test Wheels:

https://github.com/ROCm/TheRock/actions/runs/31559471913

https://github.com/ROCm/TheRock/actions/runs/31583784516

Tested by triggering Multi-Arch Release in TheRock ci: ROCm/TheRock#7383

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

testing adding test_artifacts_structure.yml

add wrappers for rockrel

Add wrappers to rockrel

ScottTodd

We need to be a bit careful adding new wrapper workflows as each one comes with maintenance cost and the risk of workflows drifting across repositories (see #49). For example, whenever the workflows change in TheRock we also need to update the stubs/wrappers here in rockrel, like on #79. We've missed that multiple times now, resulting in failed nightly release workflow runs. The issue I linked has some thoughts on how to solve for that with unit testing, documentation, or automation. If we continue down this path of adding more stub/wrapper workflows I'd like to see more of a solid test/automation plan so we're less likely to break releases going forward.

ScottTodd · 2026-08-11T15:34:10Z

Some background:

Stub workflows are needed whenever forking from the current github workflow run to a separate workflow run via uses: benc-uk/workflow-dispatch (see https://github.com/ROCm/TheRock/blob/main/.github/workflows/multi_arch_release_linux.yml). They are not needed when just using uses: ./.github/workflows

We currently dispatch for

GPU artifact testing with test_artifacts.yml

package install testing with test_native_linux_packages_install.yml

pytorch build/testing

jax build/testing

The reasons we dispatch instead of including in the current workflow run include:

Splitting release workflow runs by function: CPU build jobs in one workflow, GPU test jobs in another.

If the CPU build workflow run has flaky builds and needs retries, we can do so without waiting for or canceling GPU jobs.

GPU runners can have long queue times (12h+ at times), so we can get a "release complete" status without waiting if we only use CPU runners in the initial workflow run. Note that this prevents us from holding the release "unpublished" based on GPU test results though.

Separate monitoring per project area, GPU category, etc.: https://github.com/ROCm/rockrel/actions/workflows/multi_arch_release.yml?query=branch%3Amain shows just the rocm build, https://github.com/ROCm/rockrel/actions/workflows/test_artifacts.yml?query=branch%3Amain shows just the rocm tests (per GPU), https://github.com/ROCm/rockrel/actions/workflows/multi_arch_release_linux_pytorch_wheels.yml?query=branch%3Amain shows just the pytorch builds and tests, etc.

This particular test_artifacts_structure.yml workflow runs on CPU machines after the build, and we do run some other CPU tests during the build. I could see cases for either:

Running these tests in the current workflow run, without workflow_dispatch

Not running these tests for releases as they should already be tested by CI (weak argument)

Refactoring this and other jobs into "trigger test rocm [cpu]" (which would include this and parts of trigger_test_native_install) and "trigger test rocm [gpu]" (which could include "trigger_test_artifacts_per_family" and parts of "trigger test rocm wheels")

Keeping the current split of "feature type" -> "test workflow" (one for native linux, one for python, one for artifacts, etc.)

ScottTodd · 2026-08-11T15:35:47Z

This is not needed since the calling workflow https://github.com/ROCm/TheRock/blob/a3e3f450db81b55f53a05581ebf52a21583c1126/.github/workflows/multi_arch_build_portable_linux_pytorch_wheels.yml#L497-L519 has uses: ./.github/workflows/test_pytorch_wheels.yml

The test_pytorch_wheels_full.yml workflow does need a stub/wrapper workflow due to:

- name: Dispatch full PyTorch test workflow if: ${{ steps.cadence.outputs.dispatch == 'true' }} uses: benc-uk/workflow-dispatch@31e2b3319479a63f0ab15bf800eff9e913504e26 # v1.3.2 with: workflow: test_pytorch_wheels_full.yml

ScottTodd · 2026-08-11T15:36:28Z

This is not needed, see https://github.com/ROCm/TheRock/blob/a3e3f450db81b55f53a05581ebf52a21583c1126/.github/workflows/multi_arch_build_linux_jax_wheels.yml#L299-L306

test_jax_wheels: if: ${{ inputs.test_amdgpu_family != '' }} needs: [build_jax_wheels, generate_target_to_run] name: Test | ${{ inputs.test_amdgpu_family }} | ${{ needs.generate_target_to_run.outputs.test_runs_on }} permissions: contents: read id-token: write uses: ./.github/workflows/test_multi_arch_linux_jax_wheels.yml

ScottTodd · 2026-08-11T15:40:43Z

This is probably worth adding, unlike (currently) the pytorch and jax test jobs.

For this to be used, we'd want to dispatch the workflow here: https://github.com/ROCm/TheRock/blob/a3e3f450db81b55f53a05581ebf52a21583c1126/.github/workflows/multi_arch_release_linux.yml#L197-L299, with inputs similar to those used to test rocm wheels on CI: https://github.com/ROCm/TheRock/blob/a3e3f450db81b55f53a05581ebf52a21583c1126/.github/workflows/multi_arch_ci_linux.yml#L279-L307

That being said, we could also merge "test rocm wheels" with "test rocm native linux packages" into a single workflow to limit how many workflow files we have. It could even be a single "test rocm" workflow that would include:

artifact structure tests (CPU)

rocm wheels (python packages, GPU)

rocm native linux packages (CPU/GPU?)

rocm native windows packages (when available, CPU/GPU?)

We discussed offline. This will be useful to keep, especially as we add more tests to this workflow like tests for amdsmi, hipdnn, and rocprofiler.

To work on this you can trigger https://github.com/ROCm/TheRock/actions/workflows/test_rocm_wheels.yml directly to get familiar with the inputs and test any changes to the workflow. Then you can trigger https://github.com/ROCm/TheRock/actions/workflows/multi_arch_release.yml (in TheRock) from your branch with 1-2 GPU families per platform (maybe gfx950 on Linux due to queues for gfx942).

remove unneed workflows

ScottTodd · 2026-08-14T17:23:46Z

+on:
+  workflow_dispatch:
+    inputs:
+      amdgpu_family:
+        description: GPU family to test
+        required: true
+        type: string
+        default: "gfx94X-dcgpu"
+
+      test_runs_on:
+        description: Runner label with a compatible GPU
+        required: true
+        type: string
+        default: "linux-gfx942-1gpu-ccs-csp-ossci-rocm"


nit: can you remove the newlines between each input? We don't have them there in either:

The workflow this wraps: https://github.com/ROCm/TheRock/blob/main/.github/workflows/test_rocm_wheels.yml

Other wrappers like https://github.com/ROCm/rockrel/blob/main/.github/workflows/test_artifacts.yml

ScottTodd · 2026-08-14T17:25:16Z

+jobs:
+  test:
+    uses: ROCm/TheRock/.github/workflows/test_rocm_wheels.yml@main
+    secrets: inherit
+    with:
+      amdgpu_family: ${{ inputs.amdgpu_family }}
+      test_runs_on: ${{ inputs.test_runs_on }}
+      package_index_url: ${{ inputs.package_index_url }}
+      package_find_links_url: ${{ inputs.package_find_links_url }}
+      python_version: ${{ inputs.python_version }}
+      rocm_version: ${{ inputs.rocm_version }}
+      container_image_name: ${{ inputs.container_image_name }}
+      container_image_url: ${{ inputs.container_image_url }}
+      repository: ${{ inputs.repository }}
+      ref: ${{ inputs.ref }}


We should add quartz notify steps like those recently added to https://github.com/ROCm/rockrel/blob/main/.github/workflows/test_artifacts.yml in f962e5f here

cc @HereThereBeDragons

ScottTodd · 2026-08-14T17:27:36Z

+      ref:
+        description: Branch, tag, or SHA containing the test source
+        required: false
+        type: string
+        default: "main"


Not sure about this default, let's stick to what the other wrapper workflows do:

rockrel/.github/workflows/test_artifacts.yml

Lines 46 to 53 in cfad003

repository:

description: "TheRock repository to checkout."

type: string

default: "ROCm/TheRock"

ref:

description: "Branch, tag, or SHA to checkout in TheRock."

type: string

default: ""

rockrel/.github/workflows/test_pytorch_wheels_full.yml

Lines 48 to 55 in cfad003

repository:

description: "TheRock repository to checkout."

type: string

default: "ROCm/TheRock"

ref:

description: "Branch, tag, or SHA to checkout in TheRock."

type: string

default: ""

ScottTodd · 2026-08-14T17:30:43Z

+      amdgpu_family:
+        description: GPU family to test
+        required: true
+        type: string
+        default: "gfx94X-dcgpu"
+
+      test_runs_on:
+        description: Runner label with a compatible GPU
+        required: true
+        type: string
+        default: "linux-gfx942-1gpu-ccs-csp-ossci-rocm"


I think we can remove most of the defaults from this wrapper, to make maintenance easier. The wrappers are not dispatched by human developers that benefit from defaults, they are dispatched by release workflows that already have complete (and dynamic!) inputs.

linux-gfx942-1gpu-ccs-csp-ossci-rocm label changes frequently, so let's not add another spot that hardcodes it (source of truth for labels is https://github.com/ROCm/therock-ci-config/blob/main/runner-config.json)

https://rocm.nightlies.amd.com/whl-multi-arch/ will change with https://github.com/ROCm/TheRock/blob/main/docs/rfcs/RFC0012-Repo-Structure.md

kpack_split should be default enabled (wrapper should not have its own opinion on this, just pass inputs from the caller to the wrapped workflow)

ghcr.io/rocm/no_rocm_image_ubuntu24_04@sha256:405945a40deaff9db90b9839c0f41d4cba4a383c1a7459b28627047bf6302a26 changes frequently

remove newlines, remove most defaults, add quartz notify

HereThereBeDragons

currently we dont have quartz notify in https://github.com/ROCm/TheRock/blob/main/.github/workflows/test_rocm_wheels.yml as it is not part of the release workflow.

any idea when we will start using it?

.github/workflows/test_rocm_wheels.yml still seems a bit basic. will there be more effort to improve it? e.g. check that pip freeze actually contains your packages you are interested in? running more than rocm-sdk test? and does rocm-sdk test even test the profiler packages?

HereThereBeDragons · 2026-08-18T10:23:50Z

+          Container image reference. Leave empty to run directly on the runner
+          (required for Windows and native Linux runners).
+        type: string
+      repository:


can we please use the description we already use in other workflows?

ref: description: "Branch, tag, or SHA to checkout." type: string default: "" repository: description: "Repository to checkout. Defaults to ROCm/TheRock." type: string default: "ROCm/TheRock"

HereThereBeDragons · 2026-08-18T10:24:17Z

+permissions:
+  contents: read
+
+run-name: Test ROCm Wheels (${{ inputs.amdgpu_family }}, py${{ inputs.python_version }}, ${{ inputs.rocm_version }})


not identical to in therock:

run-name: Test ROCm Wheels (${{ inputs.amdgpu_family }}, py${{ inputs.python_version }}, ${{ inputs.rocm_version }}, ${{ inputs.test_runs_on }}) ```

HereThereBeDragons · 2026-08-18T10:25:54Z

+      package_find_links_url: ${{ inputs.package_find_links_url }}
+      python_version: ${{ inputs.python_version }}
+      rocm_version: ${{ inputs.rocm_version }}
+      kpack_split: ${{ inputs.kpack_split }}


is not defined in workflow_call in https://github.com/ROCm/TheRock/blob/main/.github/workflows/test_rocm_wheels.yml

HereThereBeDragons · 2026-08-18T10:28:13Z

+
+jobs:
+  notify_quartz_start:
+    name: "Quartz - started - test rocm wheels (rockrel)"


Suggested change

name: "Quartz - started - test rocm wheels (rockrel)"

name: "Quartz - started - test ROCm wheels (rockrel)"

HereThereBeDragons · 2026-08-18T10:28:24Z

+      ref: ${{ inputs.ref }}
+
+  notify_quartz_completed:
+    name: "Quartz - completed - test rocm wheels (rockrel)"


Suggested change

name: "Quartz - completed - test rocm wheels (rockrel)"

name: "Quartz - completed - test ROCm wheels (rockrel)"

ScottTodd · 2026-08-18T15:05:12Z

Lets figure out the workflow architecture on ROCm/TheRock#7383 before we proceed any further here. The order of this work is backwards and it should be instead:

start with getting the release workflows in TheRock to have the behavior that we want (running python package tests with an acceptable matrix of jobs)
get any changes there approved
prepare a PR like this to add wrapper workflows as necessary, get it approved
merge both PRs around the same time

Wrapper workflows serve a narrow purpose - to enable workflow_dispatch for workflows that need it across repositories. See the notes on #49 for more background.

…xed typo

changed description, edit run-name, fixed fixed typo

jonatluu added 2 commits August 6, 2026 15:36

testing adding test_artifacts_structure.yml

8222a60

Merge pull request #1 from jonatluu/users/jonatluu/wrapper

c87956f

testing adding test_artifacts_structure.yml

jonatluu marked this pull request as draft August 7, 2026 01:21

jonatluu added 2 commits August 7, 2026 14:22

add wrappers for rockrel

0339d3e

Merge pull request #2 from jonatluu/users/jonatluu/wrapper

a09e10d

add wrappers for rockrel

jonatluu changed the title ~~Add wrapper for test_artifacts_structure.yml in rockrel~~ Add wrappers to rockrel Aug 7, 2026

jonatluu added 2 commits August 7, 2026 15:30

fix formatting

4f282f0

Merge pull request #3 from jonatluu/users/jonatluu/wrapper

5787793

Add wrappers to rockrel

jonatluu marked this pull request as ready for review August 7, 2026 20:45

ScottTodd reviewed Aug 11, 2026

View reviewed changes

jonatluu added 2 commits August 12, 2026 15:55

remove unneed workflows

ac6a76a

Merge pull request #4 from jonatluu/users/jonatluu/wrapper

48f9752

remove unneed workflows

jonatluu requested a review from ScottTodd August 13, 2026 17:35

ScottTodd reviewed Aug 14, 2026

View reviewed changes

jonatluu mentioned this pull request Aug 14, 2026

Test change for test_rocm_wheels enablement in rockrel ROCm/TheRock#7383

Open

1 task

jonatluu added 2 commits August 17, 2026 17:37

remove newlines, remove most defaults, add quartz notify

617691a

Merge pull request #5 from jonatluu/users/jonatluu/wrapper

74c66ca

remove newlines, remove most defaults, add quartz notify

HereThereBeDragons reviewed Aug 18, 2026

View reviewed changes

jonatluu added 2 commits August 18, 2026 15:54

changed description to match other workflows, edit run-name, fixed fi…

b198e51

…xed typo

Merge pull request #6 from jonatluu/users/jonatluu/wrapper

437dc34

changed description, edit run-name, fixed fixed typo

	repository:
	description: "TheRock repository to checkout."
	type: string
	default: "ROCm/TheRock"
	ref:
	description: "Branch, tag, or SHA to checkout in TheRock."
	type: string
	default: ""

	name: "Quartz - started - test rocm wheels (rockrel)"
	name: "Quartz - started - test ROCm wheels (rockrel)"

	name: "Quartz - completed - test rocm wheels (rockrel)"
	name: "Quartz - completed - test ROCm wheels (rockrel)"

Uh oh!

Conversation

jonatluu commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Test Plan

Test Result

Submission Checklist

Uh oh!

ScottTodd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ScottTodd Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HereThereBeDragons left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ScottTodd commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jonatluu commented Aug 7, 2026 •

edited

Loading

ScottTodd Aug 11, 2026 •

edited

Loading