-
Notifications
You must be signed in to change notification settings - Fork 269
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
replace old torch dependency in requirements.txt #372
Conversation
[ghstack-poisoned]
ghstack-source-id: f8bf8d48cfdc59756b3d79c43ade149ee290ce17 Pull Request resolved: #372
`torch 2.2.0.dev` is too stale to be useful. For CI, since we will install nightly anyway, this avoids storing the old version in the docker image. [ghstack-poisoned]
ghstack-source-id: ed209b9ab1fb345f431255cbc750c2baad585b34 Pull Request resolved: #372
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 think we should not remove it, but rather we should bump the version to the lastest major version (i.e. 2.4.0.dev
). I feel the torch version in requirements.txt is more like a sanity check to ensure user who doesn't install nightly and have a pretty old version of torch would notice
`torch 2.2.0.dev` is too stale to be useful. For CI, since we will install nightly anyway, this avoids storing the old version in the docker image. [ghstack-poisoned]
ghstack-source-id: 8cbd62b97816ae8185b8a7e1aa9a7505f2780525 Pull Request resolved: #372
ghstack-source-id: 8cbd62b97816ae8185b8a7e1aa9a7505f2780525 Pull Request resolved: #372
agree, using 2.3.0 is a bit misleading as, torchtitan will not work with torch 2.3.0 (many of the features/apis used by torchtitan were updated/added to torch recently. we could freeze this to torch 2.4.0 after the release if we want to 'release torchtitan stably', and leave it there for some time. (but our CI would still need to install latest nightly to allow development). Also, it's a bit unfortunate that we couple together what the docker build builds and what the end-user installs. We actually don't want torch installed in the docker, since ultimately we'll have to uninstall torch and reinstall the latest nightly every time we run the test. One avenue here might be to just remove torch from requirements.txt, and do an import with try/except in torchtitan that gives some helpful info about whether the torch is missing (install it) or wrong version (which version is ok)? |
ghstack-source-id: 8cbd62b97816ae8185b8a7e1aa9a7505f2780525 Pull Request resolved: pytorch#372
ghstack-source-id: 8cbd62b97816ae8185b8a7e1aa9a7505f2780525 Pull Request resolved: pytorch#372
Stack from ghstack (oldest at bottom):
torch 2.2.0.dev
is too stale to be useful.For CI, since we will install nightly anyway, this avoids storing the old version in the docker image.
update: per @wanchaol's comment, adding 2.3.0 as it's the lastest stable version.
some comments:
2.4.0.dev
there requires--index-url https://download.pytorch.org/whl/nightly/cu121
. We don't want to specifycu121
since different people might have different cuda support. However, if we remove that, as @awgu previously explored, it will try to download everything and then select. So we'd rather let user install latest nightly.torch
version like 2.3.0 will installtriton
, whereas installing a nightly 2.4.0.dev version will installpytorch-triton
. This potentially might be the reason which caused CI failure when we removetorch
dependency inrequirements.txt
.