Cache the Modal sandbox install chain in image layers - #8536
Conversation
The dependency installs (requirements + the Torch pin) ran inside every sandbox and consumed roughly eight minutes of the fixed sandbox lifetime, which the v1 GPU suite now runs against. Bake them into content-addressed image layers instead: the requirements layer is keyed by the merge-group base SHA and the Torch layer by the preset, so warm runs skip both. The runtime pip install -r commands stay as no-op correctness guards for a candidate branch that changed a requirements file. Layer order keeps the Torch pin last, preserving the previous guarantee that a transitive dependency cannot displace the intended CUDA build. Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25fb5b7fe3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # Torch itself is pinned in the image (see _build_sandbox_image), after the requirements | ||
| # layers, so a transitive dependency cannot displace the intended CUDA build. |
There was a problem hiding this comment.
When a merge-group candidate changes any of the three requirements files to require a Torch or Torchvision version incompatible with the selected preset, the runtime pip install -r guards run after the baked Torch layer and can replace that preset. Because the former runtime reinstall is gone, pytest may then run against a different Torch/CUDA stack while still receiving the preset's --torch_ver and --cuda_ver; reapply or validate the pin after the candidate requirements are installed.
Useful? React with 👍 / 👎.
| image = modal_module.Image.from_registry(preset["image"], add_python="3.10") | ||
| image = image.run_commands( | ||
| f"python -m pip install -r {requirements_url}/requirements.txt " | ||
| f"-r {requirements_url}/requirements-dev.txt -r {requirements_url}/requirements-deepcompile.txt") | ||
| return image.pip_install(preset["torch_package"], |
There was a problem hiding this comment.
Running python3 ci/test_torch_latest.py on this tree regresses from 22/23 in the parent to 18/23: three controller tests now abort here because the fake Image.from_registry result does not implement the newly used run_commands/pip_install chaining contract, and the structural-plan test still requires the removed reinstall Torch packages command. Update the consolidated Modal fake and expectations so these controller paths remain exercised.
AGENTS.md reference: AGENTS.md:L34-L34
Useful? React with 👍 / 👎.
| env: | ||
| DS_CI_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name || github.repository }} | ||
| DS_CI_SHA: ${{ github.event.pull_request.head.sha || github.sha }} | ||
| DS_CI_BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha || '' }} |
There was a problem hiding this comment.
Commit 871569c57da8129205c742de1c13371d8677ee11 is a non-merge commit, but its commit message has no Signed-off-by trailer. This violates the repository's DCO requirement and can cause the commit to be rejected by the DCO gate; recreate it with git commit --signoff.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
2 participants
Why
The
modal-torch-latestsandbox installs its whole dependency chain at runtime, after the sandbox clock starts: requirements (~3.6 min) plus the forced Torch reinstall (~3 min) — roughly 8 of the fixed sandbox lifetime, every run. The v1 GPU suite now runs close to that lifetime (#8519's merge-queue run was SIGKILLed at the sandbox deadline with the suite at 90%), so the fixed install overhead is directly eating test budget.What
pip install -rcommands are kept unchanged as correctness guards: they are a near no-op when the baked layers are current, and install the difference when a candidate branch changed a requirements file — caching only speeds things up, it never changes the final environment.reinstall Torch packagescommand is removed (now the image layer).DS_CI_BASE_SHAis wired from the workflow (pull_request.base.sha || merge_group.base_sha), falling back to the candidate SHA on events without a base — that only lowers the cache hit rate, never correctness.Net effect: warm runs hand ~6.5 min back to pytest; cold runs (new preset / new base SHA) pay a one-time layer build that is shared across subsequent runs.
Verification
python3 ci/test_tests_fetcher.py: 16/16 passed.pre-commit run --files ci/torch_latest.py .github/workflows/modal-torch-latest.yml: all hooks passed._build_sandbox_imageandbuild_remote_commandsagainst a stub Modal module: layer order is requirements → Torch pin; the runtime chain keeps the guard commands and no longer reinstalls Torch;base_shafalls back to the candidate SHA on push events.