[WIP][CI][build] Move torch deps into requirements/torch.txt and torchlib.txt#32721
[WIP][CI][build] Move torch deps into requirements/torch.txt and torchlib.txt#32721orionr wants to merge 1 commit intovllm-project:mainfrom
Conversation
Signed-off-by: Orion Reblitz-Richardson <orionr@meta.com>
There was a problem hiding this comment.
Code Review
This pull request aims to centralize PyTorch dependencies, which is a valuable goal for simplifying maintenance. The changes successfully move torch, torchaudio, and torchvision into new requirements/torch.txt and requirements/torchlibs.txt files. However, this refactoring introduces a critical issue for ROCm builds by removing the necessary package index URL. Additionally, the new approach for CPU dependencies relies on external configuration that complicates local development setups. My review provides feedback on these points to help ensure the new dependency management strategy is robust across all platforms.
I am having trouble creating individual review comments. Click here to see my feedback.
requirements/rocm-build.txt (4-7)
Removing --extra-index-url https://download.pytorch.org/whl/rocm6.4 from this file will break ROCm builds. PyTorch for ROCm must be installed from this specific index to get the ROCm-compatible wheels. Without this, pip will likely default to PyPI and fetch the CUDA-enabled wheels, causing the build to fail. This index URL needs to be provided to the pip install command, for example in docker/Dockerfile.rocm, to ensure the correct packages are installed.
requirements/torch.txt (5-10)
Using a generic torch==2.9.1 dependency requires the build environment to provide the correct --extra-index-url for different platforms (e.g., CPU). This makes local development setup more complex and error-prone, as developers must manually provide these flags. The previous approach using platform markers within the requirements files was self-contained. To support builds without 'extra hints', as mentioned in the PR description, you would need to use markers. Given the complexity of managing all platforms in one file, consider creating platform-specific torch requirement files (e.g., torch-cpu.txt, torch-cuda.txt, torch-rocm.txt) and including the appropriate one in higher-level requirements files.
|
This pull request has merge conflicts that must be resolved before it can be |
This PR will demonstrate (not working yet) a set of changes to centralize PyTorch dependencies (
torch,torchaudio,torchvision) intorequirements/torch.txtandrequirements/torchlib.txtacross all build types and targets. This is a continuation to #30443Here is the list of todo items:
pyproject.tomlstill works withouttorchreferencesdocker/Dockerfileto eliminate torch nightly conditionals and write torequirements/torch.txtplusrequirements/torchlib.txtinstead. Flags for pip can be stored in a separate variable in base dockers.use_existing_torch.pyjust empty outrequirements/torch.txtplusrequirements/torchlib.txtand skip all other rewritesPurpose
Simplify torch requirements so they can be more easily changed for:
Test Plan
Run through Buildkite and confirm all tests pass
Test Result
N/A - still early