-
Notifications
You must be signed in to change notification settings - Fork 88
Update pyproject.toml for uv and SDK 2.26.1 #1040
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
base: main
Are you sure you want to change the base?
Conversation
tengomucho
left a comment
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.
This seems nice! I think it would be better to add some documentation about how to use uv with optimum-neuron.
JingyaHuang
left a comment
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.
Thanks for improving it! @michaelbenayoun
| "wheel", | ||
| "torch-neuron==1.13.1.2.9.74.0", | ||
| "torch==1.13.1.*", | ||
| "neuron-cc[tensorflow]==1.22.0.0", |
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 shall remove the extra which is dedicated to inf1
cc. @dacorvo @tengomucho
| "wheel", | ||
| "neuronx-cc==2.21.18209.0", | ||
| "torch-neuronx==2.8.0.2.10.13553", | ||
| "neuronx-cc==2.21.33363.0", |
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 already merged these changes last week 😉
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.
Awesome, I still had that on my branch, my bad!
I'd say let's test it and use it for a while and change the whole docs once we validated the whole thing. But yes I agree! |
dacorvo
left a comment
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.
You need to fix the unconditional import of accelerate symbols when registering training automodels
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
dacorvo
left a comment
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.
I just tried the branch and uv sync reinstalls the torch CUDA version although I have the torch CPU version installed, while uv pip install behaves as expected and keeps the torch version.
dacorvo
left a comment
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.
Just one minor change 🙏
| [[tool.uv.index]] | ||
| name = "pypi" | ||
| url = "https://pypi.org/simple" | ||
|
|
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.
| [[tool.uv.index]] | |
| name = "pytorch-cpu" | |
| url = "https://download.pytorch.org/whl/cpu" | |
| explicit = true | |
| [tool.uv.sources] | |
| torch = [ | |
| { index = "pytorch-cpu" }, | |
| ] | |
| torchvision = [ | |
| { index = "pytorch-cpu" }, | |
| ] |
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.
This will make sure we install the CPU version of pytorch
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.
Nice!
Adding it.
pyproject.toml
Outdated
| default = true | ||
|
|
||
| [[tool.uv.index]] | ||
| name = "pypi" |
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.
I do not think you need the pypi index, being the default one.
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.
Remove it then!
What does this PR do?
uvusable: we can runuv syncwithout failing now