Skip to content

deps: lock the test dependencies with hashes - #40

Open
z33b0t wants to merge 2 commits into
mainfrom
deps/lock-test-requirements
Open

deps: lock the test dependencies with hashes#40
z33b0t wants to merge 2 commits into
mainfrom
deps/lock-test-requirements

Conversation

@z33b0t

Copy link
Copy Markdown
Contributor

The cheap half of #10, which the issue itself suggests as the way in:

Given the torch interaction, locking requirements-test.txt first is a cheap way to establish the pattern.

  • requirements-test.in — hand-edited floors, unchanged content
  • requirements-test.txt — generated, 24 packages pinned exactly with hashes
  • CI installs with --require-hashes

Deliberately does not close#10

requirements.txt still carries floors. Locking it is not a mechanical repeat: torch is installed separately from the cu130 index and deliberately isn't listed, so a naive compile would resolve a PyPI torch that clobbers the cu130 wheel — and --generate-hashes needs every dependency to have artifacts on PyPI, which the git install in the Dockerfile hints isn't universally true here.

That needs a decision I'd rather you make than guess at:

  • exclude torch (and its CUDA deps) from the compile, keeping the Dockerfile's two-step install — simpler, but the lock then doesn't cover torch's own transitive tree; or
  • wire the PyTorch extra index into the compile so torch is pinned too — more complete, but the lock becomes CUDA-variant-specific and has to be regenerated whenever the cu130 pin moves.

I lean toward the first: it matches how the image already installs, and the cu130 wheel is already pinned exactly (torch==2.12.1). Tell me which and I'll do it. I've left a note in requirements.txt so the gap reads as pending rather than missed.

Verification

Fresh venv, hash-checked install, full suite:

$ pip install --require-hashes -r requirements-test.txt # exit 0
$ python scripts/test_protocol.py
all checks passed

requirements-test.in is added to server.yml's path filters so editing the source still triggers the suite.

Note for #8 / #33

This unblocks the pip half of the Dependabot config — worth adding in whichever of these lands second.

Refs #10

z33b0t added 2 commits July 29, 2026 01:17
requirements-test.txt specified only floors, so CI resolved a different tree on
every run and a compromised upstream release would have been installed on the
next one with nothing to review. Splits it the way #10 proposes:
requirements-test.in hand-edited floors
requirements-test.txt generated, 24 packages pinned exactly with hashes
CI installs with --require-hashes, so a re-uploaded or tampered-with wheel
fails the install instead of running.
This is the cheap half of #10 — the issue suggests it explicitly as the way to
establish the pattern, because the test set has no torch in it. requirements.txt
still can't be locked mechanically: torch is installed separately from the cu130
index and deliberately isn't listed, so the lock must not drag a PyPI torch in
over the cu130 wheel. Noted in requirements.txt so the asymmetry reads as
pending work rather than an oversight; #10 stays open for it.
Verified with a clean venv: `pip install --require-hashes -r
requirements-test.txt` succeeds and the full protocol suite passes against it.
requirements-test.in is added to the workflow's path filters so editing the
source re-runs the suite.
Compiled on 3.12, the resolution pinned numpy 2.5.1, which requires >=3.12 — so
the 3.11 job in the matrix couldn't install it at all:
ERROR: Could not find a version that satisfies the requirement numpy==2.5.1
Recompiled with --python-version 3.11 (the oldest version server.yml tests), so
the universal resolution emits both behind markers: numpy 2.4.6 below 3.12,
2.5.1 at or above. Documented in requirements-test.in, since the flag is easy to
leave off and the failure only shows on the oldest matrix entry.
@z33b0t

Copy link
Copy Markdown
ContributorAuthor

@z33b0t ready to merge. All 9 checks green — including Protocol suite (Python 3.11), which is the one that matters here.

That job failed on the first push and caught a real bug: I'd compiled the lock on 3.12, which pinned numpy==2.5.1 (requires >=3.12), so 3.11 couldn't install it at all. Recompiled with --python-version 3.11 so the universal resolution emits both behind markers (2.4.6 below 3.12, 2.5.1 at or above), and documented the flag in requirements-test.in — it's easy to leave off and only fails on the oldest matrix entry.

This deliberately does not close #10. It locks the test deps only. requirements.txt still carries floors, and locking it needs a decision from you — whether to exclude torch from the compile (my preference, matches how the Dockerfile already installs) or wire the PyTorch cu130 index into it. Details in the PR body; #10 stays open for that half.

Verified with a clean venv: hash-checked install succeeds and the full suite passes against it.

@z33b0t

Copy link
Copy Markdown
ContributorAuthor

@z33b0t Green and ready, but I can't merge this one — my gh token has repo but not workflow scope, and this PR modifies a file under .github/workflows/:

GraphQL: refusing to allow an OAuth App to create or update workflow
`.github/workflows/…` without `workflow` scope (mergePullRequest)

Either merge it yourself, or grant the scope and I'll finish:

gh auth refresh -h github.com -s workflow

(Oddly, #30 and #37 modified workflows and merged fine before this started refusing — so the restriction isn't applying consistently. Worth knowing in case you see it again.)

@z33b0t

Copy link
Copy Markdown
ContributorAuthor

yes, exclude torch (and its CUDA deps) from the compile

@z33b0t

Copy link
Copy Markdown
ContributorAuthor

@z33b0t Thanks — done, in #49.

Excluded from the lock: torch, triton, and the sixteen nvidia-* runtime packages, all of which the cu130 step installs first.

Kept in deliberately: torchaudio and torchmetrics. They look like they belong with torch but don't — I checked the running container and the cu130 install doesn't provide them; they arrive from PyPI as ordinary NeMo dependencies, so excluding them would leave an unpinned hole in the middle of the tree.

One thing worth knowing that fell out of doing it: torch still has to be pinned in requirements.in even though it's excluded from the output. Without the pin the resolver drifts to PyPI's newest torch (2.13.0) and pins the whole tree against a torch the image never installs. With it, the resolution reproduces the running container exactly — torchaudio 2.11.0, torchmetrics 1.9.0, numpy 2.4.6, nemo-toolkit 2.7.3.

Verified by building and running the image on the GPU box: builds under --require-hashes, torch survives as 2.12.1+cu130 with CUDA and bf16 available, and both sample clips transcribe. That last check mattered — the install upgrades cuda-bindings/cuda-pathfinder that the cu130 step brought in, which could plausibly have broken torch.


This PR (#40) is still blocked on the workflow scope — it edits server.yml's path filters. Two small things to fold in when you take it:

Sign up for freeto 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.

Pin/lock Python dependencies — requirements.txt uses >= floors

1 participant

@z33b0t