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

Issues from importing torch in setup.py #265

Open
cthoyt opened this issue Feb 2, 2022 · 9 comments
Open

Issues from importing torch in setup.py #265

cthoyt opened this issue Feb 2, 2022 · 9 comments

Comments

@cthoyt
Copy link

cthoyt commented Feb 2, 2022

As we all know, dealing with C extensions is pretty unruly and it's nice that setup.py is flexible in this regard. However, because this package's setup.py imports PyTorch, it causes all sorts of havoc with downstream code when you include torch_scatter in your own requirements. This usually leads to instructions to iteratively (manually) install PyTorch then some downstream stuff.

I think there's a solution available to alleviate this: use a pyproject.toml to specify build requirements so you can simply pip install this package without torch already installed. This was demonstrated for a different kind of package in kivy/pyjnius#594

I'll submit a PR to demonstate

cthoyt added a commit to cthoyt/pytorch_scatter that referenced this issue Feb 2, 2022
@rusty1s
Copy link
Owner

rusty1s commented Feb 2, 2022

Thanks for this issue. I understand that this works for Cython, but I am unsure what exactly happens in the PyTorch case?

For example, given the requirement.txt:

torch==1.10.0+cu102
torch-scatter

Does torch-scatter install C extensions based on torch==1.10.0+cu102 or based on which CUDA dependency?

Do you think we can make this work for PyTorch due to the differences in CUDA requirements? In my experience, forcing a specific PyTorch+CUDA combination causes more harm than good.

@cthoyt
Copy link
Author

cthoyt commented Feb 2, 2022

I think this would be up to the dependency resolver in pip (introduced in 2020) to handle - the PR demonstration I made doesn't pin the version of torch in the installation of torch-scatter, so it should respect other requirements that are given at the same time and/or the preexisting installation (meaning a potential improvement here doesn't mean anyone has to change the way they're doing anything)

@rusty1s
Copy link
Owner

rusty1s commented Feb 2, 2022

I understand that. I think most people are installing from wheel anyway, which works just fine in a single requirement.txt file or via conda. For users of pip install torch-scatter (which means manually compiling via nvcc), I think it is actually better to let installation fail in case PyTorch is not installed than to install an arbitrary PyTorch+CUDA version. This likely results in errors as the default CUDA version of PyTorch and the system CUDA do not necessarily match.

@a-gn
Copy link

a-gn commented Nov 8, 2022

I understand that. I think most people are installing from wheel anyway, which works just fine in a single requirement.txt file or via conda. For users of pip install torch-scatter (which means manually compiling via nvcc), I think it is actually better to let installation fail in case PyTorch is not installed than to install an arbitrary PyTorch+CUDA version. This likely results in errors as the default CUDA version of PyTorch and the system CUDA do not necessarily match.

Hello, we use torch-scatter in multiple deployments and we would benefit from declaring the torch build dependency in pyproject.toml.

We have a lockfile containing precise dependency versions (maintained with pip-tools). The goal is to be able to install all 100+ dependencies in one command with pip install -r requirements.txt. requirements.txt does contain the version of PyTorch/CUDA we want, but pip does not install torch before torch-scatter, because torch is not a build-time dependency of torch-scatter. Hence the install fails.

Installing in two steps would mean extracting the torch version constraint and index-url from our lockfile (along with all its dependencies), installing that, then installing the rest, including torch-scatter. You see that this complicates deployment when multiple environments and deployment systems are concerned. We had to copy the torch-scatter code in our internal repository and add a pyproject.toml there, but that complicates upgrades and requires passing that local path to pip.

I see your problem with pip silently installing a CPU version of torch when the user does not explicitly provide a torch version. I think it's more important that torch-scatter installs correctly when we give pip a complete set of dependencies, than to crash on a user error. Could we maybe add a warning in setup.py to tell the user to check which torch version is being installed?

FYI, our pyproject.toml is:

[build-system]
requires = [
  "setuptools",
  "torch",
  "wheel",
]
build-backend = "setuptools.build_meta"

I can open a PR if you're OK with this method.

@rusty1s
Copy link
Owner

rusty1s commented Nov 13, 2022

You can install via requirements.txt file by linking to the wheels. For example,

-f https://data.pyg.org/whl/torch-1.12.1+cu116.html
torch-scatter==2.0.9

@a-gn
Copy link

a-gn commented Nov 14, 2022

You can install via requirements.txt file by linking to the wheels. For example,

-f https://data.pyg.org/whl/torch-1.12.1+cu116.html
torch-scatter==2.0.9

I see, the problem is that we were using PyTorch 1.8.3 LTS and there's no matching torch-scatter build. We will see if we can upgrade to a torch version you support. Thanks!

@rusty1s
Copy link
Owner

rusty1s commented Nov 14, 2022

You can also use the wheels for 1.8.0 for PyTorch 1.8.3.

@ankitvgupta
Copy link

ankitvgupta commented May 12, 2023

I'm running into this issue via a pip install from a requirements file. Among other dependencies, we have a block in the requirements.txt that includes this:

torch ~= 2.0.0
-f https://data.pyg.org/whl/torch-2.0.0+cpu.html # for torch-scatter and torch-sparse
torch-scatter ~= 2.1.1
torch-sparse == 0.6.17

and we get the following error:

#14 139.6 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#14 147.1   WARNING: Did not find branch or tag 'ae19ce5', assuming revision or ref.
#14 300.1   error: subprocess-exited-with-error
#14 300.1   
#14 300.1   × python setup.py egg_info did not run successfully.
#14 300.1   │ exit code: 1
#14 300.1   ╰─> [6 lines of output]
#14 300.1       Traceback (most recent call last):
#14 300.1         File "<string>", line 2, in <module>
#14 300.1         File "<pip-setuptools-caller>", line 34, in <module>
#14 300.1         File "/tmp/pip-install-wselwjf3/torch-scatter_52c10a7f6acf44ae8ce45dc083e0e98b/setup.py", line 8, in <module>
#14 300.1           import torch
#14 300.1       ModuleNotFoundError: No module named 'torch'
#14 300.1       [end of output]
#14 300.1   
#14 300.1   note: This error originates from a subprocess, and is likely not a problem with pip.
#14 300.1 error: metadata-generation-failed
#14 300.1 
#14 300.1 × Encountered error while generating package metadata.
#14 300.1 ╰─> See above for output.
#14 300.1 
#14 300.1 note: This is an issue with the package mentioned above, not pip.
#14 300.1 hint: See above for details.

Do you have any recommendations for changes to the requirements.txt file to handle this?

@rusty1s
Copy link
Owner

rusty1s commented May 13, 2023

Mh, this works just fine on my end. On which OS do you run this query? Does -f https://data.pyg.org/whl/torch-2.0.0+cpu.html actually get picked up (you can confirm by running via pip install --verbose`)? It looks like it tries to install the package from source.

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

Successfully merging a pull request may close this issue.

4 participants