Skip to content

Test change for test_rocm_wheels enablement in rockrel - #7383

Open
jonatluu wants to merge 1 commit into
mainfrom
users/jonatluu/rockrel_wrapper
Open

Test change for test_rocm_wheels enablement in rockrel #7383
jonatluu wants to merge 1 commit into
mainfrom
users/jonatluu/rockrel_wrapper

Conversation

@jonatluu

Copy link
Copy Markdown
Contributor

Motivation

Corresponding change to Therock so that wrapper test_rocm_wheels.yml for rockrel can be called ROCm/rockrel#84

Issue: ROCm/rockrel#82

Technical Details

modified .github/workflows/multi_arch_release_linux.yml to call

Test Plan

PASS multi arch release Linux Trigger Test Wheels:
https://github.com/ROCm/TheRock/actions/runs/31559471913

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

Test Result

PASS

Submission Checklist

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +270 to +280
workflow: test_rocm_wheels.yml
inputs: |
{ "amdgpu_family": "${{ matrix.amdgpu_family }}",
"test_runs_on": "${{ matrix.test_runs_on }}",
"package_find_links_url": "${{ needs.build_python_packages.outputs.kpack_split == 'true' && needs.build_python_packages.outputs.package_find_links_url || format('{0}/{1}/index.html', needs.build_python_packages.outputs.package_find_links_url, matrix.amdgpu_family) }}",
"python_version": "${{ matrix.python_version }}",
"rocm_version": "${{ inputs.rocm_package_version }}",
"container_image_name": "${{ matrix.container_image_name }}",
"container_image_url": "${{ matrix.container_image_url }}",
"kpack_split": "${{ needs.build_python_packages.outputs.kpack_split }}"
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah this isn't great - the matrix is going to dispatch lots of jobs here.

I would rather have releases dispatch closer to what test_artifacts does: one workflow run per amdgpu_family, not per [amdgpu_family, container, python_version].

Need to look at the workflow architecture a bit to see how we might simplify there.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't had much time to take a more detailed look here yet.

One idea would be to add a new workflow that takes fromJSON(inputs.build_config).test_python_packages_matrix as an input and have that workflow expand the matrix itself.

CI would run that new workflow here:

test_python_packages_per_family:
needs: [build_python_packages]
name: Test Python
if: ${{ !failure() && !cancelled() && toJSON(fromJSON(inputs.build_config).test_python_packages_matrix) != '[]' }}
strategy:
fail-fast: false
matrix:
include: ${{ fromJSON(inputs.build_config).test_python_packages_matrix }}
uses: ./.github/workflows/test_rocm_wheels.yml
secrets: inherit
with:
amdgpu_family: ${{ matrix.amdgpu_family }}
test_runs_on: ${{ matrix.test_runs_on }}
# TODO: Simplify to just `needs.build_python_packages.outputs.package_find_links_url`
# once kpack split is always enabled.
package_find_links_url: >-
${{
needs.build_python_packages.outputs.kpack_split == 'true'
&& needs.build_python_packages.outputs.package_find_links_url
|| format('{0}/{1}/index.html',
needs.build_python_packages.outputs.package_find_links_url,
matrix.amdgpu_family)
}}
python_version: ${{ matrix.python_version }}
rocm_version: ${{ inputs.rocm_package_version }}
container_image_name: ${{ matrix.container_image_name }}
container_image_url: ${{ matrix.container_image_url }}
repository: ${{ inputs.repository }}
ref: ${{ inputs.ref }}

We've gone through a few iterations on the workflow matrix shape, see also

@therock-pr-bot

therock-pr-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass
Forbidden Files ✅ Pass
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled
🤖 therock-pr-bot ✅ Pass

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: TODO

Development

Successfully merging this pull request may close these issues.

2 participants