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

Add platform markers for linux only extra_install_requires #88826

Conversation

JacobHayes
Copy link
Contributor

Fixes #88049

#85097 added new extra dependencies on nvidia-*. They are linux (GPU) only packages, but were not marked as such, causing issues installing pytorch 1.13 via Poetry (and possibly other tools that follow PyPI's metadata API) on non-Linux systems. This "fixes" the issue by adding the ; platform_system = 'Linux' marker on these dependencies, but the main problem of different metadata for different wheels is a somewhat larger issue.

#85097 used ; as a delimiter for splitting the different deps, but that is the delimiter used in markers, so I changed to split on |.

@JacobHayes JacobHayes requested a review from a team as a code owner November 10, 2022 18:04
@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Nov 10, 2022
@pytorch-bot
Copy link

pytorch-bot bot commented Nov 10, 2022

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/88826

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

❌ 3 Failures

As of commit 9d8956b:

The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@ngimel ngimel added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Nov 10, 2022
Copy link

@lalmei lalmei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hmih
Copy link

hmih commented Nov 16, 2022

This PR fixes a huge nuisance for me, can I do something to help push this PR through?

@atalman atalman added the ciflow/binaries_wheel Trigger binary build and upload jobs for wheel on the PR label Nov 16, 2022
@atalman
Copy link
Contributor

atalman commented Nov 16, 2022

we need to attach ciflow/binaries_wheel label and produce the new wheel. Once this wheel is build it needs to be tested. If it works, we can ship this PR

@malfet malfet added this to the 1.13.1 milestone Nov 16, 2022
@JacobHayes
Copy link
Contributor Author

I think the test failures are flakes / unrelated (RuntimeError: functorch/test_aotdispatch failed!) and I'm not sure where to find the built wheels (I might be missing them in the GH UI?), but let me know if there's anything else I can do to help on this!

@atalman
Copy link
Contributor

atalman commented Nov 18, 2022

Tested the resulting binary installation worked:

pip install torch-1.14.0.dev20221116+cu117.with.pypi.cudnn-cp39-cp39-linux_x86_64.whl
Processing ./torch-1.14.0.dev20221116+cu117.with.pypi.cudnn-cp39-cp39-linux_x86_64.whl
Collecting networkx
  Downloading networkx-2.8.8-py3-none-any.whl (2.0 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 2.0/2.0 MB 18.2 MB/s eta 0:00:00
Collecting sympy
  Downloading sympy-1.11.1-py3-none-any.whl (6.5 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 6.5/6.5 MB 36.8 MB/s eta 0:00:00
Collecting nvidia-cudnn-cu11==8.5.0.96
  Using cached nvidia_cudnn_cu11-8.5.0.96-2-py3-none-manylinux1_x86_64.whl (557.1 MB)
Collecting typing-extensions
  Using cached typing_extensions-4.4.0-py3-none-any.whl (26 kB)
Collecting nvidia-cuda-runtime-cu11
  Using cached nvidia_cuda_runtime_cu11-11.8.89-py3-none-manylinux1_x86_64.whl (875 kB)
Collecting nvidia-cublas-cu11==11.10.3.66
  Using cached nvidia_cublas_cu11-11.10.3.66-py3-none-manylinux1_x86_64.whl (317.1 MB)
Requirement already satisfied: setuptools in ./miniconda3/envs/poetry/lib/python3.9/site-packages (from nvidia-cublas-cu11==11.10.3.66->torch==1.14.0.dev20221116+cu117.with.pypi.cudnn) (65.5.0)
Requirement already satisfied: wheel in ./miniconda3/envs/poetry/lib/python3.9/site-packages (from nvidia-cublas-cu11==11.10.3.66->torch==1.14.0.dev20221116+cu117.with.pypi.cudnn) (0.37.1)
Collecting mpmath>=0.19
  Downloading mpmath-1.2.1-py3-none-any.whl (532 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 532.6/532.6 kB 6.8 MB/s eta 0:00:00
Installing collected packages: mpmath, typing-extensions, sympy, nvidia-cuda-runtime-cu11, nvidia-cublas-cu11, networkx, nvidia-cudnn-cu11, torch


Successfully installed mpmath-1.2.1 networkx-2.8.8 nvidia-cublas-cu11-11.10.3.66 nvidia-cuda-runtime-cu11-11.8.89 nvidia-cudnn-cu11-8.5.0.96 sympy-1.11.1 torch-1.14.0.dev20221116+cu117.with.pypi.cudnn typing-extensions-4.4.0

Smoke test passed:

python smoke_test.py 
torch: 1.14.0.dev20221116+cu117-with-pypi-cudnn
torch cuda: 11.7
torch cudnn: 8500

Metadata looks good as well:

Requires-Dist: nvidia-cuda-runtime-cu11 ; platform_system == "Linux"
Requires-Dist: nvidia-cudnn-cu11 (==8.5.0.96) ; platform_system == "Linux"
Requires-Dist: nvidia-cublas-cu11 (==11.10.3.66) ; platform_system == "Linux"

@atalman
Copy link
Contributor

atalman commented Nov 18, 2022

@pytorchbot merge -f "Test failures are not related"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@varunagrawal
Copy link
Contributor

Can we cut a patch release that includes this fix? Would really help with package managers like poetry.

lsickert added a commit to inseq-team/inseq that referenced this pull request Nov 23, 2022
atalman pushed a commit to atalman/pytorch that referenced this pull request Nov 30, 2022
…8826)

Fixes pytorch#88049

pytorch#85097 added new extra dependencies on `nvidia-*`. They are linux (GPU) only packages, but were not marked as such, causing issues installing pytorch 1.13 via Poetry (and possibly other tools that follow PyPI's metadata API) on non-Linux systems. This "fixes" the issue by adding the `; platform_system = 'Linux'` marker on these dependencies, but the main problem of different metadata for different wheels is a [somewhat larger issue](pytorch#88049 (comment)).

pytorch#85097 used `;` as a delimiter for splitting the different deps, but that is the delimiter used in markers, so I changed to split on `|`.

Pull Request resolved: pytorch#88826
Approved by: https://github.com/neersighted, https://github.com/lalmei, https://github.com/malfet
atalman pushed a commit to atalman/pytorch that referenced this pull request Dec 6, 2022
…8826)

Fixes pytorch#88049

pytorch#85097 added new extra dependencies on `nvidia-*`. They are linux (GPU) only packages, but were not marked as such, causing issues installing pytorch 1.13 via Poetry (and possibly other tools that follow PyPI's metadata API) on non-Linux systems. This "fixes" the issue by adding the `; platform_system = 'Linux'` marker on these dependencies, but the main problem of different metadata for different wheels is a [somewhat larger issue](pytorch#88049 (comment)).

pytorch#85097 used `;` as a delimiter for splitting the different deps, but that is the delimiter used in markers, so I changed to split on `|`.

Pull Request resolved: pytorch#88826
Approved by: https://github.com/neersighted, https://github.com/lalmei, https://github.com/malfet
atalman added a commit that referenced this pull request Dec 6, 2022
…89924)

Fixes #88049

#85097 added new extra dependencies on `nvidia-*`. They are linux (GPU) only packages, but were not marked as such, causing issues installing pytorch 1.13 via Poetry (and possibly other tools that follow PyPI's metadata API) on non-Linux systems. This "fixes" the issue by adding the `; platform_system = 'Linux'` marker on these dependencies, but the main problem of different metadata for different wheels is a [somewhat larger issue](#88049 (comment)).

#85097 used `;` as a delimiter for splitting the different deps, but that is the delimiter used in markers, so I changed to split on `|`.

Pull Request resolved: #88826
Approved by: https://github.com/neersighted, https://github.com/lalmei, https://github.com/malfet

Co-authored-by: Jacob Hayes <jacob.r.hayes@gmail.com>
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Dec 10, 2022
…8826)

Fixes pytorch#88049

pytorch#85097 added new extra dependencies on `nvidia-*`. They are linux (GPU) only packages, but were not marked as such, causing issues installing pytorch 1.13 via Poetry (and possibly other tools that follow PyPI's metadata API) on non-Linux systems. This "fixes" the issue by adding the `; platform_system = 'Linux'` marker on these dependencies, but the main problem of different metadata for different wheels is a [somewhat larger issue](pytorch#88049 (comment)).

pytorch#85097 used `;` as a delimiter for splitting the different deps, but that is the delimiter used in markers, so I changed to split on `|`.

Pull Request resolved: pytorch#88826
Approved by: https://github.com/neersighted, https://github.com/lalmei, https://github.com/malfet
@lawrenceong
Copy link

NOTE: This fix does not address platform_system == 'Linux' and platform_machine != 'x86_64' as the nvidia-cublas-cu11 does not exist except for x86_64.

@huydhn
Copy link
Contributor

huydhn commented Dec 21, 2022

NOTE: This fix does not address platform_system == 'Linux' and platform_machine != 'x86_64' as the nvidia-cublas-cu11 does not exist except for x86_64.

This looks related to this open PR #91083, but the change doesn't seem to work out just yet

@jspaezp
Copy link

jspaezp commented Dec 21, 2022

@huydhn in my system it works setting the version to >=1.3.1, can you confirm it does not work for you in that case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/binaries_wheel Trigger binary build and upload jobs for wheel on the PR Merged open source topic: not user facing topic category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PyTorch 1.13 cannot be installed using poetry on Mac M1