ci: keep pins.json in sync with pyproject.toml; cap torch <2.12#9351
ci: keep pins.json in sync with pyproject.toml; cap torch <2.12#9351lstein wants to merge 6 commits into
Conversation
pins.json is consumed only by the Invoke Launcher (fetched at the release tag) to pick the torch wheel index for legacy installs. Nothing in-repo references it, so it silently drifted from pyproject.toml: its Linux ROCm index stayed at rocm6.3 after the torch-rocm index moved to rocm7.1, causing the launcher to install ROCm 6.3 wheels (invoke-ai#9328, launcher#131). - Add scripts/check_pins.py, which fails if any torchIndexUrl entry in pins.json differs from the matching [[tool.uv.index]] URL in pyproject.toml. - Run it from the uv-lock-checks workflow, triggered by changes to any of pyproject.toml, uv.lock, pins.json, or the check script. - Bump pins.json rocm index to rocm7.1 (matches invoke-ai#9337) so the check passes. - Bump the workflow's uv from 0.6.10 to 0.11.28: uv.lock is already lock format revision 3, which 0.6.x cannot parse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
torch 2.12.x+rocm7.1 breaks generation (invoke-ai#9328; 2.11 works, and 2.10 is what the rocm extra pins). The cap only matters for legacy (pre-6.14) launcher installs, which resolve the base range live against the pytorch wheel indexes instead of using uv.lock — with the range open to <3.0, AMD users could land on the broken 2.12.x. Lockfile-based installs are unaffected: uv.lock stays at 2.7.1 / 2.10.0+rocm7.1 (no locked versions changed, only the recorded specifier). Remove the cap once the 2.12 ROCm incompatibility is diagnosed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JPPhoto
left a comment
There was a problem hiding this comment.
invokeai/scripts/check_pins.py:32-43: The consistency check only validates entries that already exist inpins.json; it does not require every launcher-supported platform/backend entry to be present. RemovingtorchIndexUrl.linux.rocmstill printspins.json is consistent with pyproject.tomland exits 0. The launcher's schema (mainover in the launcher repo) permits backend keys to be absent, and its legacy install path only adds--indexwhen the selected entry exists. An omitted ROCm entry therefore silently falls back from the ROCm wheel index, causing AMD installations to resolve an unsuitable build or fail. This recreates the exact installation failure the new guardrail is intended to prevent. To expose this issue, add a test that removes each required CPU, CUDA, or ROCm entry in turn and asserts that the checker exits nonzero; the implementation should validate an explicit required platform/backend matrix, not merely iterate existing entries.
Resolved uv.lock conflict by regenerating from main's lock with `uv lock` rather than merging hunks. The only substantive change is the recorded `torch` specifier (>=2.7.0,<2.12); no package versions moved.
Addresses JPPhoto's review. The checker only iterated the entries that already existed in pins.json, so deleting torchIndexUrl.linux.rocm still exited 0 — recreating the exact silent-fallback failure the guardrail was meant to prevent (the launcher only passes --index when the entry exists, and its schema makes every backend key optional). - Validate pins.json against REQUIRED_BACKENDS, an explicit matrix of the platform/backend combinations the launcher can select and Invoke supports. Missing entries, missing platform sections, unknown platforms and unsupported backends are now all errors, alongside the existing URL check. - Split the logic into check_pins(), with main() taking a repo root, so the exit code is directly testable. - Add tests/test_check_pins.py: removes each required entry in turn and asserts a nonzero exit, plus stale-URL, unknown-platform and unsupported-backend cases. - pins.json: add the win32.cpu entry the matrix exposes as missing. Without it, a Windows "no dedicated GPU" legacy install gets no --index and pulls the CUDA-bundled PyPI torch wheel instead of the +cpu build. Linux already had this entry. - python-tests: trigger on scripts/** and pins.json so these tests actually run when the checker or the pins change. - Revert the uv 0.6.10 -> 0.11.28 bump in uv-lock-checks: 0.6.10 parses the revision-3 lock fine (verified), so the stated justification was wrong and the bump left this workflow out of step with python-tests.
|
Good catch — you're right, and the failure mode was exactly as you described: Explicit required matrix. REQUIRED_BACKENDS: dict[str, set[str]] = {
"win32": {"cpu", "cuda"},
"linux": {"cpu", "cuda", "rocm"},
"darwin": set(),
}It's an exact match in both directions, so four classes of drift now fail:
I derived the matrix from the launcher rather than guessing: Tests. New Also added One consequence worth flagging: the new matrix immediately caught a second missing cell, Unrelated to your review, two other changes in this push:
|
JPPhoto
left a comment
There was a problem hiding this comment.
One more thing found:
invokeai/pyproject.toml:52-54: The reported incompatibility is specificallytorch 2.12.x+rocm7.1, but the newsys_platform != 'darwin'upper bound is emitted into package metadata for every Linux and Windows installation, including CUDA, CPU-only, and Linux ARM64 environments that do not use ROCm. Direct/manual installs in those environments can therefore reject an otherwise requiredtorch>=2.12, downgrade an existing Torch installation, or fail dependency resolution; this also contradicts the PR's claim that the cap only affects legacy launcher installs. Test: resolve and smoke-test InvokeAI with Torch 2.12 on representative Windows CUDA, Windows CPU, Linux CUDA, Linux CPU, and Linux ARM64 environments, and either scope the cap to the affected legacy ROCm resolution path or document evidence that Torch 2.12 is incompatible on every capped platform.
Summary
The launcher installs InvokeAI's dependencies from two sources that nothing keeps in sync:
pins.json(fetched at the release tag, used for the torch index URL in legacy installs) andpyproject.toml/uv.lock.pins.jsondrifted — its Linux ROCm index stayed atrocm6.3afterpyproject.tomlmoved torocm7.1— so launcher installs on AMD got ROCm 6.3 wheels, which cannot work at all on RDNA4 cards (#9328). #9337 fixed the immediate URL; this PR adds the guardrail so it can't silently drift again, and caps torch below the version that breaks ROCm installs.Changes
scripts/check_pins.py: validatespins.jsonagainst an explicit required platform/backend matrix (linux: cpu/cuda/rocm,win32: cpu/cuda,darwin: none — MPS has no separate torch index). Missing entries, missing platform sections, unknown platforms, unsupported backends, and URLs that disagree with the matchingtorch-<backend>[[tool.uv.index]]inpyproject.tomlall fail the check. A missing entry matters as much as a stale one: the launcher's schema makes every backend key optional and it only passes--indexwhen the entry exists, so an absent entry silently falls back to the default PyPI index.tests/test_check_pins.py: removes each required entry in turn (and each platform section) and asserts a nonzero exit, plus stale-URL, unknown-platform and unsupported-backend cases. The parametrized cases are generated from the matrix itself so coverage can't silently shrink.pins.json: adds thewin32.cpuentry the new matrix exposes as missing. Without it a Windows "No dedicated GPU" legacy install gets no--indexand pulls the CUDA-bundled PyPI torch wheel rather than the+cpubuild;linux.cpuhas always been there.uv-lock-checksworkflow: runs the new check, triggered by changes topyproject.toml,uv.lock,pins.json, or the check script.python-testsworkflow: path filter extended toscripts/**andpins.json, so editing the checker runs its own tests.pyproject.toml+uv.lock: base torch constraint capped at<2.12on linux/windows. torch 2.12.x+rocm7.1 breaks generation ([bug]: Release 6.13.5rc1 not installing ROCm 7.1 as expected #9328; 2.11 works, and therocmextra pins 2.10.0). The cap only affects legacy (pre-6.14) launcher installs, which resolve this range live against the pytorch indexes instead of usinguv.lock— no locked versions changed (uv lock --lockedpasses; the lock diff is just the recorded specifier). Remove the cap once the 2.12 ROCm incompatibility is diagnosed.Caveat
Because the launcher fetches
pins.jsonat the release tag, none of this retroactively fixes already-tagged releases (e.g.v6.13.5.rc1carriesrocm6.3forever). It takes effect from the next tag onward.QA
python3 scripts/check_pins.pypasses on the fixed files.pytest tests/test_check_pins.py— 19 passed.uv lock --lockedpasses after the constraint change; no package versions moved in the lock.Related: #9328, #9337, invoke-ai/launcher#131
🤖 Generated with Claude Code