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

ao should have no dependencies #371

Closed
msaroufim opened this issue Jun 15, 2024 · 0 comments
Closed

ao should have no dependencies #371

msaroufim opened this issue Jun 15, 2024 · 0 comments
Labels
dependencies Pull requests that update a dependency file

Comments

@msaroufim
Copy link
Member

msaroufim commented Jun 15, 2024

As I was working on #369 I was reminded that we need to be more disciplined about our dependencies otherwise we might encounter resolution errors that would fail in unexpected ways for example #222

In a few minutes we got rid of most of them but there are 2 dependencies that are trickier to remove

  1. numpy because numpy is actually an implicit dependency in pytorch. See for example Remove top lev numpy dependency from fuzzer.py pytorch#128759 where by importing the benchmark Timer we also implicitly import the fuzzer which we don't intend to use and has a dependency on numpy

The second one is a NestedTensor dependency on numpy, this one I'm less sure sure by perhaps @jbschlosser has some insight

(fresh) [marksaroufim@devgpu003.cco3 ~]$ python
Python 3.10.14 (main, May  6 2024, 19:42:50) [GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import torchao
/home/marksaroufim/anaconda3/envs/fresh/lib/python3.10/site-packages/torch/nested/_internal/nested_tensor.py:417: UserWarning: Failed to initialize NumPy: No module named 'numpy' (Triggered internally at ../torch/csrc/utils/tensor_numpy.cpp:84.)
  values=torch.randn(3, 3, device="meta"),
  1. The other dependency is torch and this is bad because users will want their own version of torch and not whatever happens to be the latest stable release. The reason why we have this dependency is because in our setup.py we need to import torch so we can install custom cuda extensions but torch isn't a runtime dependency but a build time dependency for torchao. We also rely on setup.py which is considered to be very outdated in favor of more modern pyproject.toml

The reason why we don't do pyproject.toml today is because toml files are mostly configs and it's quite tricky to add custom logic like the logic we have in our setup.py most notably our get_extensions() function https://github.com/pytorch/ao/blob/main/setup.py#L41. torch also uses a similar setup and does not rely on pyproject.toml for similar reasons. In the past we also did have a minimal pyproject.toml just for build time dependencies but this was causing build isolation issues which were time consuming to debug in CI back when we first added custom cuda extensions #135 but also some other UX issues where torch was getting installed multiple times because per build you'd need to install torch and this also sucks if you live in a bandwidth limited area #231

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

1 participant