[TPU] move torch_xla out of tpu.txt #26938
Conversation
Signed-off-by: Chengji Yao <[email protected]>
|
Documentation preview: https://vllm--26938.org.readthedocs.build/en/26938/ |
mergify
bot
added
documentation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request moves the torch_xla dependency from requirements/tpu.txt to the installation documentation to avoid conflicts with the new tpu-inference backend. While this change is correct for manual source builds, it introduces a critical issue in the Docker build process for TPUs. The docker/Dockerfile.tpu relies on requirements/tpu.txt and will no longer install torch_xla, breaking the legacy TPU backend fallback. I've left a critical comment with a suggested fix for the Dockerfile.
![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.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
| setuptools==78.1.0 | ||
| nixl==0.3.0 | ||
| tpu_info==0.4.0 |
There was a problem hiding this comment.
Removing torch_xla[tpu, pallas]==2.8.0 from the TPU requirements means the Docker build that uses this file (docker/Dockerfile.tpu) will now uninstall torch_xla and never reinstall it. The Dockerfile still runs pip uninstall -y torch torch_xla torchvision before pip install -r requirements/tpu.txt, so the resulting image no longer contains torch_xla and any TPU runtime code that imports it will fail at startup. Consider either leaving torch_xla in this requirements file or explicitly installing it again inside the Dockerfile.
Useful? React with 👍 / 👎.
|
@yaochengji as the bot says, will this affect the dockerfile and should we update that too? |
Signed-off-by: Chengji Yao <[email protected]>
Signed-off-by: Chengji Yao <[email protected]>
@mgoin thanks for reminding me! Yes, we need to make the dockerfile for the torch_xla backend correct. |
2 participants
Purpose
Move the legacy torch_xla requirement out of tpu.txt to avoid conflicts with the new tpu-inference backend.