Repository navigation
Exclude implicit PyTorch targets by release - #9549
Conversation
✅ 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. |
ScottTodd
left a comment
There was a problem hiding this comment.
A bit hacky but okay given
- the temporary nature of this (pytorch support is coming to each release branch)
- how we want to get gfx1250-strict builds running continuously to guard against regressions during bringup
The main design constraint here this might not satisfy is if gfx1250-strict support is added to some of the pytorch release branches but not all. This code is all-or-nothing right now.
Broad ROCm families can include targets that some PyTorch releases do not support. Select implicit-target exclusions per platform and PyTorch ref so those releases can keep building supported family members. Exclude gfx1250-strict for Linux release/2.13, release/2.14, release/2.15, and nightly while retaining explicit target requests. Custom refs and Windows currently have no implicit-target exclusions. Pass the policy through CI and release matrices. Direct dispatches resolve auto through the same policy lookup. Filter expanded targets before device-extra grouping, preserving input order and deduplication. Progress #8278 Test Plan: - python build_tools/github_actions/tests/expand_amdgpu_families_test.py - python build_tools/github_actions/tests/configure_pytorch_release_matrix_test.py - python build_tools/github_actions/tests/configure_multi_arch_ci_test.py - python build_tools/github_actions/tests/configure_pytorch_test_matrix_test.py - python build_tools/github_actions/tests/workflow_dispatch_inputs_test.py Test Result: - Passed: all five suites (226 tests and 83 subtests) - Skipped: one existing CI configuration test
8eb3f8b to
8e32536
Compare
Thanks for the review Scott. The above is indeed a constraint we would need follow up rather quickly. I rewrite the implementation and now have the exclude defined in the pytorch version matrix. Please re-review. |
| # Excluded implicit targets remain available when requested directly. | ||
| exclude_implicit_targets="${EXCLUDE_IMPLICIT_TARGETS}" | ||
| if [[ "${exclude_implicit_targets}" == "auto" ]]; then | ||
| exclude_implicit_targets=$(python build_tools/github_actions/configure_pytorch_release_matrix.py \ | ||
| --platform linux \ | ||
| --resolve-excluded-implicit-targets "${PYTORCH_GIT_REF}") | ||
| fi |
There was a problem hiding this comment.
This branch is awkward but I see the reasoning behind it.
I had some similar code back with the pytorch manifest generation where we want to support all of:
- ci.yml / release.yml entry points that define the matrix completely upfront and pass explicit inputs all the way through
- release_pytorch.yml entry point that either gets an explicit matrix or derives it on its own
- build_pytorch.yml entry point that either gets an explicit matrix or derives it on its own
One way I handled it there was to have an optional step that runs the script or an optional job that runs at the start of the workflow, instead of interleaving into existing steps like this. Then you can see the default code flow more clearly.
There was a problem hiding this comment.
Yeah, it is not awesome but does its job. Will try to not forget and follow up.
## Motivation ROCm/pytorch `release/2.13` and `release/2.14` now support `gfx1250-strict`, but TheRock still removes that target when `gfx125X-dcgpu` is expanded for those refs. Normal multi-architecture PyTorch builds therefore compile and package only `gfx1250` in the shared `amd-torch-device-gfx1250` wheel. This supersedes #9748. The branch now lives in this repository, per the branch creation policy, so the `release/2.14` wheel workflow can be dispatched from it. ISSUE ID : #8278 Related: - ROCm/pytorch#3895 - ROCm/pytorch#3907 - #9549 - #9748 (superseded) ## Technical Details - Stop excluding `gfx1250-strict` from implicit target expansion for PyTorch `release/2.13` and `release/2.14`. - Keep the `release/2.15` and `nightly` exclusions unchanged. - Update release-matrix and multi-arch CI tests to lock in the per-ref policy. - Add an `expand_build_configs` test asserting that the non-empty `release/2.15` exclusion reaches the generated PyTorch matrix, addressing review feedback on #9748. ## Risk Assessment Level 3. This expands the default PyTorch build target set for two release branches, but does not change the wheel-splitting or package-ownership logic. ## Device / Architecture Coverage The change is scoped to `gfx1250-strict`. Unit tests verify that 2.13 and 2.14 include it while 2.15 and nightly continue to exclude it. Multi-Arch CI covers the `release/2.13` build; `release/2.14` requires a direct dispatch of `multi_arch_build_portable_linux_pytorch_wheels.yml` from this branch. No GPU tests were run for this policy-only change. ## Test Plan Run the focused release-matrix, multi-arch CI, and family-expansion tests, confirm the new test fails when the `release/2.15` exclusion is dropped, then run pre-commit over the changed files. ## Test Result - `configure_pytorch_release_matrix_test.py`: 17 passed. - `configure_multi_arch_ci_test.py`: 183 passed, 1 skipped. - `expand_amdgpu_families_test.py`: 18 passed. - With `resolve_excluded_implicit_targets` patched to return an empty string, the new test fails with `'' != 'gfx1250-strict'`. - Pre-commit checks over all changed files passed. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md. Generated with Cursor --------- Co-authored-by: Cursor <cursoragent@cursor.com>
Motivation
Broad ROCm families can include targets that some PyTorch releases do
not support. Select implicit-target exclusions per platform and PyTorch
ref so those releases can keep building supported family members.
Progress #8278
Technical details
Exclude gfx1250-strict for Linux release/2.13, release/2.14,
release/2.15, and nightly while retaining explicit target requests.
Custom refs and Windows currently have no implicit-target exclusions.
Pass the policy through CI and release matrices. Direct dispatches
resolve auto through the same policy lookup. Filter expanded targets
before device-extra grouping, preserving input order and deduplication.
Test Plan
Test Result
Submission Checklist
Generated with Codex