Skip to content

ci(pytorch): derive numpy test pin from vendored torch checkout#6423

Draft
lucbruni-amd wants to merge 5 commits into
mainfrom
users/lucbruni-amd/pytorch-derive-numpy-pin
Draft

ci(pytorch): derive numpy test pin from vendored torch checkout#6423
lucbruni-amd wants to merge 5 commits into
mainfrom
users/lucbruni-amd/pytorch-derive-numpy-pin

Conversation

@lucbruni-amd

@lucbruni-amd lucbruni-amd commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Motivation

Follow-up / alternative to the review discussion on #6400. The unpinned numpy
in external-builds/pytorch/requirements-test.txt broke the non-full PyTorch
test workflow once a newer NumPy tightened uint8 casting (an upstream OpInfo
bug, pytorch/pytorch#189267). #6400 fixes it by hand-copying upstream's
per-Python numpy pins, but there is silent drift potential.

This prototype removes the manual copy: it derives numpy from the checked-out
torch's own .ci/docker/requirements-ci.txt at install time, so we always use
what upstream validates for the exact ref under test.

Note: the full test workflow already installs the entire requirements-ci.txt
(since #3705), so it is already covered; this only closes the gap in the lean
non-full test_pytorch_wheels.yml without pulling upstream's ~75-package set.

Technical Details

  • external-builds/pytorch/derive_test_requirements.py: prints the requirement
    lines for the requested packages (default numpy) from the vendored checkout's
    .ci/docker/requirements-ci.txt, preserving environment markers; errors if the
    file is missing.
  • requirements-test.txt: drop bare numpy (now derived).
  • test_pytorch_wheels.yml: after installing the lean reqs, pipe the derived
    pins into pip.

It auto-tracks the torch ref (nightly → nightly's numpy, release/2.9 → 2.9's
numpy), so there is nothing to re-sync and nothing to drop when
pytorch/pytorch#189267 is fixed upstream.

Test Plan

Local verification (below); in CI, the non-full PyTorch test jobs install the
derived numpy and the threshold uint8 tests pass.

Test Result

Ran locally against the vendored checkout:

  • default → nightly's 4 per-Python numpy lines
  • marker selection for py3.12 → numpy==1.26.2
  • --requirements-ci pointed at ROCm/pytorch release/2.9numpy==2.0.2 / 2.1.2 (per-branch tracking)
  • missing file → clean error (exit 2)

Submission Checklist

Instead of hand-copying upstream's per-Python numpy pins into
requirements-test.txt (which drifts and is per-torch-branch), derive
them at install time from the checked-out torch's own
.ci/docker/requirements-ci.txt via derive_test_requirements.py, and wire
it into the non-full PyTorch test workflow.

Prototype / follow-up to the review discussion on #6400.
Relates to pytorch/pytorch#189267.
@therock-pr-bot

therock-pr-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
🌿 Branch Name ✅ Pass
📝 PR Title/Description ✅ Pass
Forbidden Files ✅ Pass
🧪 Unit Test ✅ Pass
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled
🤖 therock-pr-bot ✅ Pass

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

@therock-pr-bot

therock-pr-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown

Pre-commit check failed

pre-commit failed

Please run locally:

  • python -m pip install pre-commit
  • pre-commit install
  • pre-commit run --all-files --show-diff-on-failure

This repo uses .pre-commit-config.yaml.

@therock-pr-bot

therock-pr-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the lean (non-full) PyTorch wheel test workflow to avoid manual NumPy pin drift by deriving the NumPy requirement directly from the checked-out PyTorch source tree’s .ci/docker/requirements-ci.txt, matching what upstream validates for the exact ref under test.

Changes:

  • Remove the bare numpy entry from external-builds/pytorch/requirements-test.txt and document that NumPy is derived at test time.
  • Add external-builds/pytorch/derive_test_requirements.py to extract and print requirement lines (default: numpy) from the vendored PyTorch checkout’s CI requirements file.
  • Update .github/workflows/test_pytorch_wheels.yml to install the derived NumPy pins after installing the lean test requirements.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
external-builds/pytorch/requirements-test.txt Drops bare numpy and documents that NumPy is now derived per torch ref.
external-builds/pytorch/derive_test_requirements.py Adds a helper to extract pinned requirement lines (e.g., NumPy) from the checked-out PyTorch repo’s CI requirements file.
.github/workflows/test_pytorch_wheels.yml Installs derived NumPy pins in the non-full wheel test workflow to avoid upstream drift.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread external-builds/pytorch/derive_test_requirements.py Outdated
Comment thread external-builds/pytorch/derive_test_requirements.py Outdated
Comment thread .github/workflows/test_pytorch_wheels.yml Outdated
lucbruni-amd and others added 3 commits July 8, 2026 13:13
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lucbruni-amd

Copy link
Copy Markdown
Contributor Author

Possibly superseded by #6384.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: TODO

Development

Successfully merging this pull request may close these issues.

2 participants