Repository navigation
fix(RocmPackageHelper): resolve and pin a matched ROCm torch/torchvision pair - #1769
Conversation
…ion pair AMD's torchvision wheels currently declare no torch version constraint, so this auto-discovers, resolves, and pins the matched torch/torchvision pair defensively, covering cases where the matched build is only published as a pre-release (as torchvision 0.29.0a0 was for torch 2.14.0), the newest torch has no matching torchvision yet, or the resolution is otherwise broken in the future. Covers both `stable` and `nightly` repositories. - Resolve the pair up front from the ROCm index: query torch (finals on stable, pre-releases on nightly) and torchvision (--pre so the matched alpha is visible), then pick the newest torch that has a paired torchvision, walking down to older torch builds when the newest has none - On the nightly channel, pin both to the same dated snapshot (matching local label) so torch and torchvision come from the same build instead of pip pairing different dates independently - Add RocmTorchVersionResolver for PEP 440-style release parsing, the torch -> torchvision pairing (torchvision minor = torch minor + 15 for the 2.x line), pair/snapshot selection, and base-version dedupe of local labels - Install torch and torchvision with explicit versions in a single pip command, falling back to an unpinned, --pre-allowing install when no paired set can be resolved - Extract the shared upgrade/reinstall/user-override flags into ApplyRocmTorchInstallOptions - Import torchvision in the post-install verification so a mismatched pair fails the install, and surface stderr when verification produces no output - Add RocmTorchVersionResolverTests for local-label stripping, release-pair parsing, the pairing map, matched pre-release selection, pair walk-down, snapshot matching, and local-label dedupe
mohnjiles
left a comment
There was a problem hiding this comment.
Thanks for this one! Resolving the pair from the index and pinning both is a really nice fix, and importing torchvision in the verification step means a bad pair fails the install instead of the first generation :3 One thing I'd like sorted before merging, inline, plus a small question below.
small q: the fallback now adds --pre, which is a bit less conservative than before on the stable channel (it'd allow a pre-release torch too if one ever shows up on the stable index). Since we only land there when resolution failed, is --pre needed for that path, or could the fallback stay as it was? Not a blocker, just curious what you had in mind.
…m torch specs
- Extract BuildRocmTorchArgs and key the pinned specs ("torch" / "torchvision") so an override replaces the pin instead of being appended alongside it
- Scope the fallback --pre to the nightly channel, keeping the stable fallback conservative
…de replacement Covers the arg building from the previous commit, including the regression where a pinned spec survived a user override and produced an unsatisfiable command. - Assert the resolved pair is emitted as torch[device-gfxXXXX]==X and torchvision[device-gfxXXXX]==X alongside torchaudio - Assert a user "torch" override replaces the pinned spec instead of being appended next to it - Same for a user "torchvision" override
Co-Authored-By: Lykos (Opus 5.5) <noreply@lykos.ai>
|
Pushed a small fixup on top (1347b1b): |
ionite34
left a comment
There was a problem hiding this comment.
Looks great, thanks! Picking the newest torch that has a matching torchvision, falling back to the old install on any failure, and failing the install early on a bad pair is a nice shape. Both of Mohn's earlier comments are fixed with regression tests. The cancel fixup we pushed (1347b1b) is the only change on top 🐺
AMD's ROCm torch and torchvision wheels carry no cross version constraints in their metadata, so pip resolves the two independently and nothing prevents it from selecting a mismatched pair. On Windows ROCm that produced
torch 2.14.0withtorchvision 0.28.0, which fails atimport torchvision. The gap is general: whenever the newest torch and the newest torchvision do not line up on the index, for example when the matched build is offered only as a pre-release that pip skips, pip can install an incompatible pair and has no way to detect it.This resolves the torch/torchvision pair from the ROCm index up front and installs both with explicit versions, so the installed pair is always the newest one that is actually available together and cannot drift.
Changes
torch(finals on the stable channel, pre-releases on nightly) andtorchvision(with--preso the matched alpha is visible).torchthat has a pairedtorchvision, walking down to older torch builds when the newest has none.RocmTorchVersionResolverfor PEP 440-style release parsing, the torch to torchvision pairing (torchvision minor = torch minor + 15 for the 2.x line), pair/snapshot selection, and base-version dedupe of local labels.torchandtorchvisionwith explicit versions in a single pip command, falling back to an unpinned,--pre-allowing install only when no paired set can be resolved.ApplyRocmTorchInstallOptions.torchvisionin the post-install verification so a mismatched pair fails the install, and surface stderr when verification produces no output.Behavior
torch 2.14.0+torchvision 0.29.0a0(stable)2.14.0+0.29.0a02.15.0present but no torchvision0.302.14.0+0.29.x...a20261007, torchvision only...a20261006...a20261006torch/torchvisionwith--pre(logged)No
--preis used on the normal install path; the exact pins install pre-releases without it.Testing
dotnet test ./StabilityMatrix.Tests/StabilityMatrix.Tests.csproj: 638 passed, 21 skipped, 0 failed.RocmTorchVersionResolverTestscovering local-label stripping, release-pair parsing, the pairing map, matched pre-release selection, pair walk-down, snapshot matching, and local-label dedupe.Notes
torchaudiois intentionally left unpinned. AMD publishes it independently and it is not version-matched to torch on this index.pip index versions, which is platform-filtered, so per-OS nightly build outages and asymmetric platform lag are handled automatically.0.29.0a0publish as a mistake and may add a torch constraint to torchvision's metadata. This PR change is defensive for any potential future mismatches in the repo that can cause package PyTorch breakage during install/update.Related ROCm/TheRock