-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Reinstate existing torch script #24729
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request reinstates a script to remove 'torch' from requirement files. The script correctly identifies and removes the lines, but the file modification is not atomic, which could lead to data loss if the script is interrupted. I've suggested a safer and more robust implementation that replaces the entire script logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to update the docs (the uv build isolation thing doesn't always work), but we can always do that in another PR
The uv approach seems to only work when you exclusively use uv to manage your virtual environments (we have reports of it not working with conda envs). This PR is a followup to vllm-project#24729 and vllm-project#24303 Signed-off-by: Richard Zou <zou3519@gmail.com>
Doc fix over at #24892 |
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Forward fix to re-enable the old method before #24303