Skip to content

feat(vllm): install ROCm 10.1 wheels from staging with a cp314 runtime (ROCMAI-439) - #471

Open
juhovainio wants to merge 9 commits into
mainfrom
rocmai-439-vllm-101-staging-cp314
Open

juhovainio wants to merge 9 commits into
mainfrom
rocmai-439-vllm-101-staging-cp314

Conversation

@juhovainio

@juhovainio juhovainio commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Enables a ROCm 10.1 vLLM staging install using a cp314 Python runtime (ROCMAI-439).

  • cp314 interpreter detection (apps/rocm/src/therock.rs): ROCm 10.x's vLLM/flash-attn/amd-aiter wheels are cp314-only, unlike every earlier SDK version's cp312 wheels. Interpreter selection previously only switched to cp314 when a stable version was explicitly pinned at ROCm >= 10 (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, so canonical_auto_select_needs_next_python now peeks the channel's live package listing to catch those cases too. ensure_uv_venv also 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.
  • vLLM ROCm 10.x discovery fixes (engines/vllm/src/install.rs):
    • The final install of vllm/flash-attn/amd-aiter resolves full dependencies, which pulls in vllm's own plain-PyPI transitive deps (e.g. lm-format-enforcer) that AMD's index doesn't host. uv treats a 403 from any configured index as fatal, and a CLI --extra-index-url can't opt a non-pytorch index into ignore-error-codes tolerance alongside it, so a generated uv.toml (write_vllm_index_uv_config, passed via --config-file) now does that instead.
    • 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 a forced torch realignment immediately afterward to restore the pinned build.
    • 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.
  • docs/vllm.md: documents the install.rs-side fixes above.
  • Test coverage:
    • An offline unit test in install.rs guards the config-file 403 fix and the torch realignment sequencing end-to-end against fake uv/python shims.
    • therock-next-09 (tests/e2e-cucumber) is dropped from @nightly to 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-09 is 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 -- --check
  • cargo clippy --workspace --all-targets --all-features
  • therock-next-09 live GPU lane (runs per-PR on self-hosted runners)

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>
@juhovainio juhovainio changed the title feat(vllm): install ROCm 10.1 wheels from staging with a cp314 runtime feat(vllm): install ROCm 10.1 wheels from staging with a cp314 runtime (ROCMAI-439) Sep 30, 2026
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 juhovainio left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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:

  1. The new interpreter provisioning now runs before the channel/arch validation it used to run after, so a doomed request (wrong channel, or a --family with no exact arch match) pays for a full uv python install of 3.14 before failing instead of failing fast.
  2. The managed-Python cache's identity check doesn't account for the fact that version now 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.

Comment thread apps/rocm/src/therock.rs
Comment thread apps/rocm/src/therock.rs
Comment thread apps/rocm/src/therock.rs
Comment thread engines/vllm/src/install.rs
Comment thread apps/rocm/src/therock.rs
Comment thread engines/vllm/src/install.rs
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>
@juhovainio
juhovainio marked this pull request as ready for review October 1, 2026 11:57
@juhovainio
juhovainio requested a review from a team as a code owner October 1, 2026 11:57

@jussielo-amd jussielo-amd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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:

  • cp314 is hardcoded independently in therock.rs's python_requirement() and each install.rs discover 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants