Skip to content
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

Merged
merged 3 commits into from
May 29, 2024

Conversation

tianyu-l
Copy link
Contributor

@tianyu-l tianyu-l commented May 29, 2024

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:

  1. Putting 2.4.0.dev there requires --index-url https://download.pytorch.org/whl/nightly/cu121. We don't want to specify cu121 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.
  2. Installing a stable torch version like 2.3.0 will install triton, whereas installing a nightly 2.4.0.dev version will install pytorch-triton. This potentially might be the reason which caused CI failure when we remove torch dependency in requirements.txt.

tianyu-l added a commit that referenced this pull request May 29, 2024
ghstack-source-id: f8bf8d48cfdc59756b3d79c43ade149ee290ce17
Pull Request resolved: #372
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label May 29, 2024
`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]
tianyu-l added a commit that referenced this pull request May 29, 2024
ghstack-source-id: ed209b9ab1fb345f431255cbc750c2baad585b34
Pull Request resolved: #372
Copy link
Contributor

@wanchaol wanchaol left a 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]
tianyu-l added a commit that referenced this pull request May 29, 2024
ghstack-source-id: 8cbd62b97816ae8185b8a7e1aa9a7505f2780525
Pull Request resolved: #372
@tianyu-l tianyu-l merged commit c4ea0a8 into gh/tianyu-l/13/base May 29, 2024
5 checks passed
tianyu-l added a commit that referenced this pull request May 29, 2024
ghstack-source-id: 8cbd62b97816ae8185b8a7e1aa9a7505f2780525
Pull Request resolved: #372
@tianyu-l tianyu-l deleted the gh/tianyu-l/13/head branch May 29, 2024 23:59
@tianyu-l tianyu-l changed the title remove old torch dependency in requirements.txt replace old torch dependency in requirements.txt May 30, 2024
@wconstab
Copy link
Contributor

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)?

tianyu-l added a commit to tianyu-l/torchtitan_intern24 that referenced this pull request Aug 16, 2024
ghstack-source-id: 8cbd62b97816ae8185b8a7e1aa9a7505f2780525
Pull Request resolved: pytorch#372
philippguevorguian pushed a commit to YerevaNN/YNNtitan that referenced this pull request Aug 17, 2024
ghstack-source-id: 8cbd62b97816ae8185b8a7e1aa9a7505f2780525
Pull Request resolved: pytorch#372
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants