external-builds/pytorch: add a torch-only ROCm ASAN build mode - #7313
external-builds/pytorch: add a torch-only ROCm ASAN build mode#7313sa-faizal wants to merge 2 commits into
Conversation
❌ PR Check — Action Required
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
You can stack your PR either with https://docs.github.com/en/pull-requests/how-tos/stacked-pull-requests or by changing the base branch for this PR to that other branch, fyi. Both require creating branches in the shared repository, which is what we recommend for AMD employees anyways: https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md#branch-creation-policy. |
) ## Motivation Follow-up to #7294, where the request was for an off-the-shelf markdown link checker rather than fixing broken links one instance at a time. ## Technical Details Three changes: 1. `.pre-commit-config.yaml` — the `lychee` block exactly as written in [#7294](#7294 (comment)), plus `stages: [manual]`. 2. `external-builds/pytorch/README.md:199` — one leading `/`. The sibling link two lines below at `:201` already has it. 3. `CONTRIBUTING.md` — the manual invocation, added to the existing `#### pre-commit checks` example block. **No `exclude` is needed.** `.pre-commit-config.yaml:4` already carries a global `exclude: "third_party/"`, which covers `build_tools/third_party/`. That is the whole of the 3-vs-4 discrepancy: my 4 came from running the `lychee` binary directly over the tree, which applies no pre-commit file filtering; the 3 came through `pre-commit`, which does. My earlier claim that the `implib` link "needs an exclude or the hook stays red" was wrong. ### Why `stages: [manual]` Because two of the three links it reports cannot be fixed here — open pull requests rewrite both lines — so at the default stage the existing `.github/workflows/pre-commit.yml` job would fail on every PR until they land. | Link | Open PR covering that line | Status | | --- | --- | --- | | `docs/development/README.md:24` | #7253, hunk `@@ -17,11 +17,11 @@` deletes that exact line | left alone | | `docs/development/workflow_outputs.md:305` | #4685, hunk `@@ -295,15 +295,15 @@` rewrites that exact line | left alone | | `external-builds/pytorch/README.md:199` | none of the 8 open PRs on the file; nearest hunks are #5439 `190-195` and #7313 `180-185` | fixed here | Hunk ranges are from each PR's combined diff against its own merge base. The scan covered all 481 open PRs. ### What `stages: [manual]` actually does It skips the binary install, not the hook repo clone. Each row is a fresh `PRE_COMMIT_HOME`: | Command | hook repo cloned | `lychee` binary installed | hook runs | | --- | --- | --- | --- | | `pre-commit install-hooks` | yes, 7.4 MB | no | no | | `pre-commit run` | yes, 7.4 MB | no | no | | `pre-commit run --all-files` | yes, 7.4 MB | no | no | | `pre-commit run --hook-stage manual --all-files` | yes | yes, 31 MB total, 109 s | yes | | *without* `stages: [manual]`, `pre-commit run --all-files` | yes | yes, 31 MB total, 121 s | yes | So the `cargo` step you had to approve is skipped on commit and in CI; the 7.4 MB `git clone` of `lycheeverse/lychee` still happens on any first `pre-commit` invocation, including `install-hooks`. Separately: the `lychee` hook id bootstrapped fine here on a clean cache — it fetches `cargo-binstall` and a prebuilt binary, no pre-existing `cargo` needed. My earlier report that it failed to bootstrap was a network restriction on my side, not the hook. ## Test Plan `pre-commit` 4.6.2, Python 3.12.10, Windows 11, at `47947a1`. Each run used an isolated `PRE_COMMIT_HOME`. Linux and macOS were **not** exercised. ## Test Result Default stage, whole config, whole tree — what CI runs. `lychee` is absent from the list, and the run is green: ``` $ pre-commit run --all-files Trim Trailing Whitespace..................................................Passed Fix End of Files..........................................................Passed Check Yaml................................................................Passed Check JSON................................................................Passed Check for merge conflicts.................................................Passed Check for added large files...............................................Passed Mixed line ending.........................................................Passed black.....................................................................Passed clang-format..............................................................Passed mdformat..................................................................Passed No-tabs checker...........................................................Passed Lint GitHub Actions workflow files........................................Passed Forbid secrets, certificates, keys, and executables...(no files to check)Skipped Enforce *_test.py naming for build_tools tests........(no files to check)Skipped exit=0 ``` Manual stage, after the `external-builds/pytorch/README.md` fix — 3 errors down to 2, and the OK count moves 506 → 507: ``` $ pre-commit run --hook-stage manual --all-files lychee Issues found in 2 inputs. Find details below. [docs/development/README.md]: [ERROR] .../docs/development/test_runner_info.md (at 24:3) | File not found. [docs/development/workflow_outputs.md]: [ERROR] .../build_tools/github_actions/upload_pytorch_manifest.py (at 305:3) | File not found. 🔍 2238 Total 🔗 1196 Unique ✅ 507 OK 🚫 2 Errors 👻 1729 Excluded 🔍 269 Total 🔗 241 Unique ✅ 55 OK 🚫 0 Errors 👻 214 Excluded ``` The `third_party` A/B, deleting only line 4 of `.pre-commit-config.yaml` and changing nothing else: ``` # line 4 present (as shipped) — 3 errors, implib is not even an input 🔍 2238 Total 🔗 1197 Unique ✅ 506 OK 🚫 3 Errors 👻 1729 Excluded # line 4 deleted — the 4th error appears [build_tools/third_party/implib/doc/ReduceLibraryInterface.md]: [ERROR] .../build_tools/third_party/implib/doc/tests/multilib/run.sh (at 32:138) | File not found. 🔍 1177 Total 🔗 596 Unique ✅ 107 OK 🚫 1 Error 👻 1069 Excluded ``` If you want the class actually gated rather than merely available, the existing `pre-commit` job takes it in three lines — but only once the two links above are resolved, since it would be red until then: ```yaml - uses: pre-commit/action@2c7b380 # v3.0.1 with: extra_args: --hook-stage manual --all-files lychee ``` Happy to send that as a follow-up after #7253 lands. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md.
Stack: 2 of 2, after #7312.
For the Phase 2-only review diff, compare
draft/asan-wheels-phase1-rocm...draft/asan-wheels-phase2-pytorch.Summary
Add the integrated Phase 2 production-builder mode for a torch-only ROCm ASAN wheel built from the isolated Phase 1 package set:
gfx942:xnack+target;--no-index,--no-build-isolation, exact versioning, and the selector'sdeviceextra;CMAKE_ARGS;LD_PRELOADto the post-install Torch sanity subprocess only.ASAN mode opts into the generic portable-RPATH contract from #7210; its parser/archive-gate implementation is deliberately not duplicated here.
Why this is a draft
gfx942:xnack+that should move into workflow configuration;ROCm/rocm-systemsmust land before the final split; andReal-input evidence
The prototype consumed the Phase 1
10.1.0+asan.20260807index and a HIPified PyTorch checkout based ondcf02e51371deb3050899643462349c1941e8979.2.14.0a0+rocm10.1.asan.20260807;No local runner, wheel, dependency wheelhouse, generated AOTriton images, PyTorch HIPIFY output, submodule pointer, or validation artifact is included.
Tests
The three portable-RPATH tests from the integrated prototype were excluded because #7210 owns that implementation and its focused coverage.
Dependencies and final acceptance
ROCm/rocm-systemskpack RECORD fixAfter those dependencies land, rebuild rather than patch the preserved wheels and require zero absolute RPATH/RUNPATH components, strict offline installation, successful gfx942 tensor execution (
1240.0), and zero downgraded ASAN findings.