Skip to content

Centralize torch-family version pins in torch_pin.py#19155

Open
mergennachin wants to merge 1 commit intomainfrom
centralize_torch_pin
Open

Centralize torch-family version pins in torch_pin.py#19155
mergennachin wants to merge 1 commit intomainfrom
centralize_torch_pin

Conversation

@mergennachin
Copy link
Copy Markdown
Contributor

@mergennachin mergennachin commented Apr 27, 2026

Centralize torch-family version pins in torch_pin.py

torch_pin.py is now the single source of truth for torch + the three
domain libraries (vision/audio/codec). It exposes a CHANNEL field
(nightly/test/release), the four version constants, NIGHTLY_VERSION,
and helpers — torch_spec() / torchaudio_spec() / torchcodec_spec() /
torchvision_spec() emit the right pip spec, torch_index_url_base()
returns the right wheel index, and torch_branch() / torchaudio_branch()
/ torchvision_branch() derive the upstream release/M.N branch from each
package's version. Every consumer — install_requirements.py, the two
install_pytorch.sh / utils.sh shell helpers, test_model_e2e.sh,
test_wheel_package_qnn.sh, the moshi/mimi install_requirements.sh, the
update_pytorch_pin.py script, and the weekly bump workflow — reads
through these helpers instead of re-encoding the version strings.

Switching to a release candidate is now a one-line change (CHANNEL =
"test") plus bumping the four version constants. The header in
torch_pin.py walks through the procedure.

update_pytorch_pin.py imports CHANNEL / NIGHTLY_VERSION / torch_branch
directly (no more regex parsing of the file). For nightly it pins to
an immutable SHA looked up by date; for test/release it writes
torch_branch() (e.g. "release/2.12") into
.ci/docker/ci_commit_pins/pytorch.txt so git checkout follows
cherry-picks as they land.

The weekly-pytorch-pin-bump workflow is guarded on CHANNEL == "nightly"
and uses an in-place re.sub on NIGHTLY_VERSION (the previous
printf '...' > torch_pin.py would have clobbered the new constants
and helpers).

test/test_torch_pin.py covers all three channels, all four specs, and
the release/M.N branch derivation.

Co-authored-by: Claude noreply@anthropic.com

Copilot AI review requested due to automatic review settings April 27, 2026 15:35
@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented Apr 27, 2026

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19155

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

❌ 4 New Failures, 5 Cancelled Jobs, 3 Pending, 4 Unrelated Failures

As of commit ab8742b with merge base 3a62fac (image):

NEW FAILURES - The following jobs have failed:

CANCELLED JOBS - The following jobs were cancelled. Please retry:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

BROKEN TRUNK - The following jobs failed but was present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 27, 2026
@github-actions
Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@mergennachin mergennachin requested a review from rascani April 27, 2026 15:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 makes torch_pin.py the single source of truth for torch + domain-library version pins and wheel index selection across the repo, so scripts/CI don’t re-encode version strings and channel-specific URLs.

Changes:

  • Introduces torch_pin.py helpers for channel-aware pip specs (*_spec()) and index URLs (torch_index_url_base()), plus release-branch derivation helpers.
  • Updates multiple install/test scripts and the pin-bump automation to consume torch_pin.py instead of hardcoding versions/URLs.
  • Adds pytest coverage validating behavior for nightly, test, and release channels.

Reviewed changes

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

Show a summary per file
File Description
torch_pin.py Adds centralized channel + version constants and helpers to generate pip specs, index URLs, and release branches.
test/test_torch_pin.py Adds tests covering spec/url/branch generation across channels.
install_requirements.py Switches torch/vision/audio spec + index URL selection to torch_pin helpers.
examples/models/moshi/mimi/install_requirements.sh Uses torch_pin to install torchcodec from the correct channel/index independent of CWD.
.github/workflows/weekly-pytorch-pin-bump.yml Guards weekly bump on CHANNEL == nightly and updates NIGHTLY_VERSION in-place.
.github/scripts/update_pytorch_pin.py Imports pin/channel info from torch_pin and writes either a nightly SHA or release branch ref.
.ci/scripts/utils.sh Chooses audio/vision refs based on channel (PyTorch pins for nightly; release branches otherwise).
.ci/scripts/test_wheel_package_qnn.sh Installs torch using torch_pin spec + index URL rather than reconstructing version strings.
.ci/scripts/test_model_e2e.sh Installs torchcodec via torch_pin helpers instead of a hardcoded nightly spec.
.ci/docker/common/install_pytorch.sh Mirrors channel-aware audio/vision ref selection for source builds.

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

Comment on lines +45 to +55
python -c "
import re, pathlib
p = pathlib.Path('torch_pin.py')
p.write_text(re.sub(
r'^NIGHTLY_VERSION\s*=\s*\".*\"$',
'NIGHTLY_VERSION = \"${{ steps.nightly.outputs.version }}\"',
p.read_text(),
count=1,
flags=re.MULTILINE,
))
"
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

The inline python -c block is indented inside the quoted string, which will be passed to Python with leading spaces and typically raises IndentationError: unexpected indent. Use an unindented heredoc (python - <<'PY' ... PY) or a single-line python -c command so the workflow reliably updates NIGHTLY_VERSION.

Copilot uses AI. Check for mistakes.
Comment thread .github/scripts/update_pytorch_pin.py
Comment thread .github/scripts/update_pytorch_pin.py
Comment thread install_requirements.py
@mergennachin mergennachin force-pushed the centralize_torch_pin branch from 0206298 to e9d0e26 Compare April 27, 2026 15:41
Copilot AI review requested due to automatic review settings April 27, 2026 15:53
@mergennachin mergennachin force-pushed the centralize_torch_pin branch from e9d0e26 to 6c23dd0 Compare April 27, 2026 15:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


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

Comment thread .ci/scripts/utils.sh Outdated
Comment thread .github/scripts/update_pytorch_pin.py
Comment thread install_requirements.py
Comment thread install_requirements.py
Comment thread torch_pin.py
Comment thread .github/workflows/weekly-pytorch-pin-bump.yml
@mergennachin mergennachin force-pushed the centralize_torch_pin branch 2 times, most recently from eedfeae to 0d60c16 Compare April 27, 2026 16:23
Copilot AI review requested due to automatic review settings April 27, 2026 16:23
@mergennachin
Copy link
Copy Markdown
Contributor Author

@claude Review this PR

@claude
Copy link
Copy Markdown

claude Bot commented Apr 27, 2026

Claude finished @mergennachin's task in 3m 2s —— View job


Review of PR #19155 — Centralize torch-family version pins in torch_pin.py

  • Read all changed files
  • Analyze torch_pin.py (the new source of truth)
  • Analyze consumers (install scripts, CI scripts, workflows)
  • Analyze tests
  • Post review

Overall: This is a well-structured consolidation. Having one file own channel selection, version constants, pip specs, index URLs, and branch derivation eliminates a real class of drift bugs. The three-channel design (nightly/test/release) with the --no-cache-dir RC stale-cache mitigation is thoughtful. A few issues worth addressing before merge:


Issues

1. do_not_use_nightly_on_ci() will reject test/release wheels (.ci/scripts/utils.sh:244-258)

This guard asserts +git in the torch version string, which is only true for source builds. When CHANNEL is test or release, CI installs published wheels (e.g. 2.12.0 or 2.12.0rc1) — the assertion will fail and kill the job. This function needs to be channel-aware: either skip the check when CHANNEL != "nightly", or broaden the check to accept wheel-installed versions.

Called from setup-linux.sh:27 and setup-macos.sh:140, so this will break CI the moment someone flips to test.

Fix this →

2. No CHANNEL validation in torch_pin.py (line 18)

A typo like CHANNEL = "nigthly" would silently produce wrong specs and URLs (the _spec helper falls through to the non-nightly path; torch_index_url_base would emit .../nigthly). Adding a module-level assertion against {"nightly", "test", "release"} catches this at import time rather than at install time with a cryptic pip error.

Fix this →

3. Weekly bump workflow doesn't verify the re.sub matched (.github/workflows/weekly-pytorch-pin-bump.yml:47-57)

If the NIGHTLY_VERSION line format ever changes (quotes, naming, etc.), the re.sub will silently write back the original content and create a no-op PR. Using re.subn and asserting replacements == 1 would surface this immediately. Copilot flagged this too — their suggested code looks correct.

4. TORCH_VERSION:0:7 short-hash assumption in utils.sh (line 132-134)

When CHANNEL != "nightly", pytorch.txt now contains a branch name (e.g. release/2.12) instead of a SHA. The ${TORCH_VERSION:0:7} used to construct the cached wheel URL will produce release — a nonsensical cache key. This path is only reached in the nightly source-build branch (the test/release branch returns early at line 111), so it's currently safe. But it's fragile — worth adding a comment noting this code only runs for nightly where TORCH_VERSION is always a SHA.

5. PR description mentions torchaudio_branch() / torchvision_branch() — they don't exist

The PR description says torch_pin.py exposes torchaudio_branch() and torchvision_branch(), but the actual code only has torch_branch(). Minor docs-only issue — update the PR description to match reality.


Suggestions (non-blocking)

A. Rename use_pytorch_nightly parameter vestiges in install_requirements.py

The internal variable was nicely renamed to install_pinned_version, but the CLI flag is still --use-pt-pinned-commit and the top-level NOTE (line 32-33) still references NIGHTLY_VERSION. Consider a follow-up to update the note to say "If you change torch_pin.py, re-run this script" — more accurate for all three channels.

B. test_torch_pin.py fixture doesn't fully isolate

The test fixture mutates pin.CHANNEL on the live module and reloads after yield. This works but means tests aren't truly isolated — if a test crashes before yield, the module stays mutated for subsequent tests. Using monkeypatch.setattr would be more robust:

@pytest.fixture
def pin(monkeypatch):
    import torch_pin
    yield torch_pin, monkeypatch

C. Consider adding a torchcodec_spec test

test_specs_drop_suffix_off_nightly tests all four specs (good), but test_channel_resolution only checks torch_spec. Since torchcodec is the newest addition and is consumed by test_model_e2e.sh and mimi/install_requirements.sh, a quick parametrized check for it would add confidence.

D. Docker build.sh: single Python invocation is good, but fragile to parse

The sed -n '1p' through '5p' parsing of the multi-line Python output (.ci/docker/build.sh:98-103) works but is position-dependent. If someone adds a print or a warning leaks to stdout, the parsing shifts. A slightly more robust approach would be to output key=value pairs and source them, or use separate -c calls (at the cost of 5 Python startups).


What looks good

  • The --no-cache-dir for RC wheels is a nice detail — RC versions get re-uploaded and pip caching causes real pain there.
  • The Docker build arg forwarding (build.shDockerfileinstall_pytorch.sh) is clean and avoids needing torch_pin.py inside the Docker build context.
  • update_pytorch_pin.py importing directly from torch_pin instead of regex-parsing is a clear improvement.
  • The _REPO_ROOT path resolution in update_pytorch_pin.py correctly handles invocation from any CWD.
  • The install_pytorch_and_domains() early-return pattern in both utils.sh and install_pytorch.sh is easy to follow.

Summary: The centralization is sound and will pay for itself the first time someone needs to flip to a release candidate. Issues #1 (do_not_use_nightly_on_ci) and #2 (CHANNEL validation) should be fixed before merge — #1 is a latent CI breakage. The rest are hardening improvements.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

install_requirements.py:80

  • In the pip install call, torch is installed with --extra-index-url pointing at the PyTorch wheel index. For CHANNEL="test"/"release", the pinned version (e.g. torch==2.12.0) also exists on PyPI, so pip can satisfy the requirement from PyPI first (typically CPU-only), ignoring the CUDA-specific index computed by determine_torch_url(...). To ensure the intended CPU/CUDA wheels are used when not on nightly, install the torch-family packages via --index-url <torch_url> (or split into a separate pip invocation for torch/vision/audio using --index-url) rather than relying on --extra-index-url with an exact version that is also present on PyPI.
    # Determine the appropriate PyTorch URL based on CUDA delegate status
    torch_url = determine_torch_url(torch_index_url_base())

    # pip packages needed by exir.
    TORCH_PACKAGE = [
        # Default: install the specific pinned version from the channel selected
        # in torch_pin.py. With --use-pt-pinned-commit, pass plain "torch" and
        # let pip resolve its default (CI's source-build is already installed).
        (torch_spec() if install_pinned_version else "torch"),
    ]

    # Install the requirements for core ExecuTorch package.
    # `--extra-index-url` tells pip to look for package versions on the
    # provided URL if they aren't available on the default URL.
    subprocess.run(
        [
            sys.executable,
            "-m",
            "pip",
            "install",
            *_NO_CACHE_DIR_FLAG,
            "-r",
            "requirements-dev.txt",
            *TORCH_PACKAGE,
            "--extra-index-url",
            torch_url,
        ],

install_requirements.py:140

  • Same issue for torchvision/torchaudio installs: with CHANNEL="test"/"release" the pinned versions are available on PyPI, so using only --extra-index-url can cause pip to pull CPU wheels from PyPI instead of the CUDA-specific index returned by determine_torch_url(...). Consider installing these domain libraries in a separate pip call using --index-url <torch_url> (and only using PyPI as an extra index for non-torch deps) so the selected PyTorch index is actually authoritative.
def install_optional_example_requirements(install_pinned_version):
    # Determine the appropriate PyTorch URL based on CUDA delegate status
    torch_url = determine_torch_url(torch_index_url_base())

    print("Installing torch domain libraries")
    DOMAIN_LIBRARIES = [
        (torchvision_spec() if install_pinned_version else "torchvision"),
        (torchaudio_spec() if install_pinned_version else "torchaudio"),
    ]
    # Then install domain libraries
    subprocess.run(
        [
            sys.executable,
            "-m",
            "pip",
            "install",
            *_NO_CACHE_DIR_FLAG,
            *DOMAIN_LIBRARIES,
            "--extra-index-url",
            torch_url,
        ],

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

Comment thread torch_pin.py
Comment on lines +34 to +59
def torch_spec() -> str:
return _spec("torch", TORCH_VERSION)


def torchaudio_spec() -> str:
return _spec("torchaudio", TORCHAUDIO_VERSION)


def torchcodec_spec() -> str:
return _spec("torchcodec", TORCHCODEC_VERSION)


def torchvision_spec() -> str:
return _spec("torchvision", TORCHVISION_VERSION)


def torch_index_url_base() -> str:
if CHANNEL == "release":
return "https://download.pytorch.org/whl"
return f"https://download.pytorch.org/whl/{CHANNEL}"


def torch_branch() -> str:
# PyTorch uses "release/M.N" branches; derive from the pinned version.
# Used by update_pytorch_pin.py to write into pytorch.txt for test/release.
return f"release/{TORCH_VERSION.rsplit('.', 1)[0]}"
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

The PR description says torch_pin.py exposes torchaudio_branch() / torchvision_branch() (and mentions branch derivation for the domain libs), but this module currently only defines torch_branch(). Either add the missing branch helpers or update the PR description/header comments so the documented API matches what’s actually available.

Copilot uses AI. Check for mistakes.
@mergennachin mergennachin force-pushed the centralize_torch_pin branch from 0d60c16 to ee7449d Compare April 27, 2026 18:58
Copilot AI review requested due to automatic review settings April 27, 2026 19:04
@mergennachin mergennachin force-pushed the centralize_torch_pin branch from ee7449d to 321bc6d Compare April 27, 2026 19:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.


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

Comment thread .ci/scripts/utils.sh
Comment thread .ci/scripts/utils.sh
Comment thread .ci/docker/common/install_pytorch.sh
Comment thread .github/workflows/weekly-pytorch-pin-bump.yml
Comment thread .ci/scripts/test_wheel_package_qnn.sh
@mergennachin mergennachin force-pushed the centralize_torch_pin branch from 466fc10 to addc31f Compare April 27, 2026 19:19
Copilot AI review requested due to automatic review settings April 27, 2026 19:40
@mergennachin mergennachin force-pushed the centralize_torch_pin branch from addc31f to 0ac917d Compare April 27, 2026 19:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.


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

torch_pin.py is now the single source of truth for torch + the three
domain libraries (vision/audio/codec). It exposes a CHANNEL field
(nightly/test/release), the four version constants, NIGHTLY_VERSION,
and helpers — torch_spec() / torchaudio_spec() / torchcodec_spec() /
torchvision_spec() emit the right pip spec, torch_index_url_base()
returns the right wheel index, and torch_branch() derives the
upstream release/M.N branch from TORCH_VERSION. Every consumer —
install_requirements.py, the two install_pytorch.sh / utils.sh shell
helpers, test_model_e2e.sh, test_wheel_package_qnn.sh, the moshi/mimi
install_requirements.sh, the update_pytorch_pin.py script, and the
weekly bump workflow — reads through these helpers instead of
re-encoding the version strings.

For test/release the docker image build (install_pytorch.sh) and the
CI bring-up path (utils.sh) install the published wheels directly via
pip --index-url, replacing the source-build-from-git-branch dance
inherited from the original 2.11 RC pin (#18287). Nightly keeps the
source-build path so CI catches upstream regressions.

This commit is a no-op for the active pin: CHANNEL = "test" with
TORCH_VERSION = "2.11.0", TORCHAUDIO_VERSION = "2.11.0",
TORCHCODEC_VERSION = "0.11.0", TORCHVISION_VERSION = "0.26.0" produces
the same wheels (from /whl/test/cpu) that origin/main currently
hardcodes. Switching channels later (e.g. to nightly post-release)
becomes a one-line change in torch_pin.py plus bumping the version
constants.

update_pytorch_pin.py imports CHANNEL / NIGHTLY_VERSION / torch_branch
directly (no more regex parsing of the file). For nightly it pins to
an immutable SHA looked up by date; for test/release it writes
torch_branch() (e.g. "release/2.11") into
.ci/docker/ci_commit_pins/pytorch.txt so git checkout follows
cherry-picks as they land.

The weekly-pytorch-pin-bump workflow is guarded on CHANNEL == "nightly"
and uses an in-place re.sub on NIGHTLY_VERSION (the previous
\`printf '...' > torch_pin.py\` would have clobbered the new constants
and helpers).

test/test_torch_pin.py covers all three channels, all four specs, and
the release/M.N branch derivation.

Co-authored-by: Claude <noreply@anthropic.com>
@mergennachin mergennachin force-pushed the centralize_torch_pin branch from 0ac917d to ab8742b Compare April 27, 2026 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/cuda ciflow/metal CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants