feat(vllm): install ROCm 10.1 wheels from staging with a cp314 runtime (ROCMAI-439) - #471
juhovainio wants to merge 9 commits into
Conversation
ROCm 10.x vLLM, flash-attn, and amd-aiter wheels are cp314-only, and ROCm 10.1's frameworks are staged on a separate index than 10.0's production one. The discover route now selects an index, torch index, and Python tag per row keyed on major.minor rather than major alone, so 10.0 and 10.1 resolve through distinct rows instead of sharing one. Torch is now discovered like the other three packages instead of pinned as a literal, since ROCm 10.1's local version rotates too fast to express as a fixed pin. Every resolved pin carrying a +rocmX.Y local version is checked against the SDK's own line before installing, so an index serving more than one ROCm line can't silently install the wrong one's wheel. ROCm CLI now derives the required Python tag (cp312 vs cp314) from the requested source layout before the interpreter is chosen, since that is knowable from the request itself ahead of resolution. A wrong-tag interpreter is rejected with a message naming the required tag, split out from the unrelated venv-creation failure it used to be reported as. The same gate now also applies when reusing a manifest-recorded interpreter on update, closing a bypass that let a stale cp312 venv carry into a layout change. Ref ROCMAI-439 Signed-off-by: Juho Vainio <juho.vainio@amd.com>
python_gate_rejects_the_wrong_tag_without_blaming_the_venv called the cfg(unix)-gated write_fake_python_with_venv helper but only carried a runtime guard, which doesn't stop Windows from trying to compile the call to a function that doesn't exist there. Signed-off-by: Juho Vainio <juho.vainio@amd.com>
juhovainio
left a comment
There was a problem hiding this comment.
I reviewed this against ROCMAI-439 and the repo's documented conventions (crate-layering, module organization, DCO/test requirements). This is delivered as a comment rather than a formal review because I'm the PR author -- self-review restrictions block REQUEST_CHANGES/APPROVE from the same account as the PR, not because these are advisory.
Two of the inline notes are real bugs worth fixing before merge:
- The new interpreter provisioning now runs before the channel/arch validation it used to run after, so a doomed request (wrong channel, or a
--familywith no exact arch match) pays for a fulluv python installof 3.14 before failing instead of failing fast. - The managed-Python cache's identity check doesn't account for the fact that
versionnow depends on layout, so switching between a canonical install and a ROCm 10.x one will thrash the cache on every switch instead of keeping both interpreters around.
The rest are smaller: a naming deviation from the ticket worth a line in the description, one duplicated-parsing-logic smell, a documentation/primitive-obsession note that isn't blocking, and a stale test comment contradicted by this PR's own doc-comment update two hunks over.
One thing I checked and didn't flag: this PR bundles the ROCm 10.1 staging wheels and the cp312-to-cp314 interpreter switch in one commit. They're tightly coupled (10.1's wheels only exist for cp314), so I don't think this needs splitting, but calling it out in case a reviewer expected two smaller commits.
No scope creep, no missing tests I could find, docs/vllm.md and the e2e feature file are both kept in sync with the behavior change.
A request select_source_layout would reject anyway (wrong channel, or a grouped family with no exact arch) used to fail before any work happened. Since the interpreter requirement started depending on layout, the Python provisioning step ran first, so the same doomed request now pays for a full `uv python install` of 3.14 before failing. Hoist the validation ahead of interpreter resolution. Also note the managed-Python manifest's single-slot cache thrashes when alternating between canonical and Next layouts, and fix a test comment left describing the pre-PR major-only matching semantics. Signed-off-by: Juho Vainio <juho.vainio@amd.com>
requested_source_layout only routes to SourceLayout::Next (and its cp314 python_requirement) when a stable version is pinned at ROCm >= 10. A plain auto-select or a nightly build-date/prerelease pin on the canonical channel can also land on a ROCm >= 10 build without ever going through Next-layout routing, so python_requirement stayed on cp312 and the install failed against cp314-only wheels. canonical_auto_select_needs_next_python peeks the channel's live rocm package listing and checks the highest matching candidate's major version, so these routes get the cp314 interpreter too. ensure_uv_venv also now checks the existing venv's wheel-compatibility python tag against what's required, not just that python --version succeeds, so a stale cp312 venv from before this fix gets recreated instead of silently reused once cp314 is required. Signed-off-by: Juho Vainio <juho.vainio@amd.com>
…nstall The ROCm 10.x discover route's final install resolves vllm/flash-attn/ amd-aiter with full dependencies, which also pulls in vllm's own plain-PyPI transitive dependencies (e.g. lm-format-enforcer) that AMD's index doesn't host. uv treats a 403 from any configured index as fatal by default, and a CLI --extra-index-url can't opt a non-pytorch index into ignore-error-codes tolerance alongside it: that only works via a uv.toml [[index]] entry passed with --config-file, and --config-file and --extra-index-url for the same URL don't merge. write_vllm_index_uv_config now generates that uv.toml so the install can fall through to PyPI instead of failing on the first 403. That same full-dependency resolve can pull in an unconstrained torch from PyPI, undoing the exact ROCm-pinned torch installed just before it. The install now runs an unconditional, forced torch realignment immediately afterward to put the pinned build back. Torch itself is still installed in its own call scoped to only the torch index, with --no-deps: mixing indexes makes uv probe torch under the vllm index too (403s fatally on AMD's CDN for a path it doesn't serve), and torch's wheel metadata pins an exact rocm[libraries] dependency that may not match the SDK's actual (e.g. nightly) version. tensorizer is no longer independently pinned: it has no ROCm-specific build, and vllm's own wheel metadata already declares an exact dependency on it, so a second independent pin risked conflicting with that. Signed-off-by: Juho Vainio <juho.vainio@amd.com>
Drop @nightly: install_vllm_rocm10_discover always runs its 403-tolerant config-file install and forced torch realignment regardless of which discover-table row is newest, so this is the live regression guard for both ROCMAI-439 fixes as well as the discovery mechanism itself, not just nightly-cadence coverage. Also extend the scenario to serve the model it just installed and send it a chat completion, reusing the same serve/chat steps every other engine-canary scenario uses. serve-vllm-inference (model_serving.feature) doesn't cover this route: its runtime comes from a plain unpinned install, never the ROCm 10 preview source. Without this, a wheel that installs cleanly but can't actually load or answer a request would pass every assertion in this scenario and still be unservable. Signed-off-by: Juho Vainio <juho.vainio@amd.com>
jussielo-amd
left a comment
There was a problem hiding this comment.
Reviewed the vLLM ROCm 10.1 staging-wheel install path in detail, with particular focus on the supply-chain angle given it installs from a "staging" index.
Supply chain: verified independently (not from the PR description) — both the 10.1 frameworks-prereleases.amd.com and existing 10.0 frameworks.amd.com index URLs are legitimate AMD-owned HTTPS infrastructure following AMD's established naming convention, no typosquat/insecure patterns. Every package resolves to an exact pinned version via uv pip install --dry-run before install — never unpinned. New guards (ensure_rocm_local_version_matches, ensure_discover_python_tag) fail closed if the staging index's mixed ROCm 10.0/10.1 wheels or the interpreter's ABI tag don't match the target. No CI/workflow changes, no new egress or secrets.
Quality: cargo test (1458 tests across rocm-engine-vllm/rocm-core/rocm), cargo fmt --check, cargo clippy, and xtask check-crate-edges all clean.
Two minor non-blocking notes for a follow-up, not blocking this PR:
cp314is hardcoded independently intherock.rs'spython_requirement()and eachinstall.rsdiscover row — nothing enforces they stay in sync if a future ROCm point release changes the CPython tag.- Small hard-wrap artifact in
docs/vllm.md(cosmetic only).
Approving — solid, well-tested change with good defensive guards for the exact cross-contamination risks a staging multi-version index introduces.
…e step `And a model is being served on GPU` inherited `Then` from the preceding step, but that step text is only registered as a `#[given(...)]` step definition. Cucumber found no match in the `Then` collection and silently skipped the step, which the reconciliation layer then flagged as an unexpected failure on every self-hosted GPU lane (MI300X, MI350P, Strix Halo WSL2). Every other scenario reusing this step precedes it with `Given`, matching the registration. Signed-off-by: Juho Vainio <juho.vainio@amd.com>
…stall The full-dependency resolve in install_vllm_rocm10_discover can replace torchvision/torchaudio with unconstrained PyPI builds ABI-incompatible with the realigned torch, causing RuntimeError: operator torchvision::nms does not exist at serve time. Snapshot whatever torchvision/torchaudio the SDK install already wrote via `uv pip freeze` before the resolve runs, then restore them alongside torch in the final realignment step. Signed-off-by: Juho Vainio <juho.vainio@amd.com>
…h realign uv keeps --extra-index-url pip-compatible: it is checked after the implicit default PyPI index, not before it. Torch's discovered pin always carries a +rocmX.Y local version absent from PyPI, so it was never affected, but the torchvision/torchaudio pins captured from `uv pip freeze` carry plain version numbers that can collide with a same-numbered public PyPI release. uv was silently installing that generic wheel over AMD's build, reproducing the torchvision::nms ABI mismatch the realignment step exists to prevent even after the prior fix. Both calls are single-pin --no-deps installs with nothing legitimate for PyPI to supply, so --index-url (replacing the default index outright) closes the loophole. Signed-off-by: Juho Vainio <juho.vainio@amd.com>
Summary
Enables a ROCm 10.1 vLLM staging install using a cp314 Python runtime (ROCMAI-439).
SourceLayout::Next). A plain auto-select or a nightly build-date/prerelease pin on the canonical channel can also resolve to a ROCm >= 10 build without going through that routing at all, socanonical_auto_select_needs_next_pythonnow peeks the channel's live package listing to catch those cases too.ensure_uv_venvalso now checks an existing venv's wheel-compatibility python tag against what's required, so a stale cp312 venv from before this fix isn't silently reused once cp314 is required.lm-format-enforcer) that AMD's index doesn't host.uvtreats a 403 from any configured index as fatal, and a CLI--extra-index-urlcan't opt a non-pytorch index intoignore-error-codestolerance alongside it, so a generateduv.toml(write_vllm_index_uv_config, passed via--config-file) now does that instead.install.rsguards the config-file 403 fix and the torch realignment sequencing end-to-end against fakeuv/pythonshims.therock-next-09(tests/e2e-cucumber) is dropped from@nightlyto a per-PR lane, since the discovery mechanism and both fixes above run on every install regardless of which discover-table row is newest.therock-next-09is extended to also serve the model it just installed and send it a chat completion, reusing existing serve/chat steps. The existing vLLM serving canary (serve-vllm-inference) installs via a plain unpinned SDK install and never exercises the ROCm 10.x discovery route, so without this a discovered wheel that installs cleanly but can't actually load or answer a request would pass undetected.Test plan
cargo test -p rocm-engine-vllm(install.rs unit tests, including the new config-file/torch-realignment regression test)cargo test -p rocm-core --lib(therock.rs cp314-detection and venv-recreation unit tests)cargo test --test feature_naming(e2e feature-file naming/sequencing contract)cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-featurestherock-next-09live GPU lane (runs per-PR on self-hosted runners)