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

Updating missing build dependency in pyproject.toml #1680

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

loadams
Copy link

@loadams loadams commented Jun 13, 2023

No description provided.

@calebho
Copy link

calebho commented Jun 13, 2023

I think you need to add torch as well because torch is also a build dependency. This dep isn't so straightforward because if you want to build the CUDA extensions, you'd need to use a different index (i.e. the --index-url bit in the PyTorch installation instructions for pip + Linux + CUDA XX.YY) depending on what CUDA you're using. I think the README would also need to be updated to explain this

@crcrpar
Copy link
Collaborator

crcrpar commented Jun 13, 2023

That sounds also reasonable. fwiw example commands of README have --no-build-isolation, which could make situations simpler.

@loadams
Copy link
Author

loadams commented Jun 13, 2023

@calebho - correct, just updating packaging leaves torch as a missing dependency, but there must be a way to do this without having each person modify the --index-url for their specific version, right?

@calebho
Copy link

calebho commented Jun 13, 2023

@loadams Not sure; you'd have to test it out yourself. @crcrpar's comment about --no-build-isolation ignores build dependencies entirely but shifts the responsibility to users to install the correct build dependencies beforehand

@Quentin-Anthony
Copy link

I propose keeping packaging and removing torch in this PR. This works for me and several others across systems and environments.

I can't see the DeepSpeed build issue in #1680 (comment) anymore, but I suspect that's an edge case for which apex can just recommend --no-build-isolation for in the README? I can add a line in the README install section to that effect if everyone's onboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants