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

Remove all dependencies except torch #369

Merged
merged 21 commits into from
Jun 17, 2024
Merged

Remove all dependencies except torch #369

merged 21 commits into from
Jun 17, 2024

Conversation

msaroufim
Copy link
Member

@msaroufim msaroufim commented Jun 15, 2024

  1. got rid of the packaging dependency in favor of importlib
  2. numpy dependency was implicit with benchmark Timer, so also removed that but needed to ignore a nested tensor warning
  3. our only real requirements left is torch
  4. everything else should be a dev dependency for testing

Tested locally import torchao which works fine and CI is working fine

Packaging logic also works fine

>>> from importlib.metadata import version
>>> version("torch")
'2.3.1'
>>> version("torch") >= "2.3.0.dev"
True
>>> version("torch") >= "2.4.0.dev"
False

We can also get rid of torch as a dependency, work is tracked here #371

CI failure is unrelated and will get fixed when we merge #377 so will rebase this PR before merge

Copy link

pytorch-bot bot commented Jun 15, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit 10cd6b0 with merge base 664f073 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 15, 2024
@msaroufim msaroufim changed the title clean up req.txt [wip] clean up req.txt Jun 15, 2024
@msaroufim msaroufim changed the title [wip] clean up req.txt Remove most dependencies Jun 15, 2024
@msaroufim msaroufim requested review from drisspg, jerryzh168 and janeyx99 and removed request for drisspg June 15, 2024 20:41
@msaroufim msaroufim changed the title Remove most dependencies Remove all dependencies except torch Jun 15, 2024
@msaroufim msaroufim requested a review from andrewor14 June 16, 2024 01:41
@msaroufim msaroufim requested a review from jcaip June 16, 2024 01:44

# For prototype features and benchmarks
bitsandbytes #needed for testing triton quant / dequant ops for 8-bit optimizers
matplotlib
pandas
fire # QOL for commandline scripts
tabulate # QOL for printing tables to stdout
numpy # mx benchmarks leverage benchmark timer which uses numpy
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there another timer that could be used?
That being said I can't remember if numpy is a dependency of torch

Copy link
Member Author

@msaroufim msaroufim Jun 16, 2024

Choose a reason for hiding this comment

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

numpy is not supposed to be a dependency of torch but I've seen that assumption broken multiple times - I remember chatting about this with @malfet at some point too

That said the reason why this is needed is because of pytorch/pytorch#128759 which fixes the problem but only in 2.5+ lol

And yes we could just use another timer and drop this dependency completely even from our dev dependencies - although I really need to be writing docs tomorrow so would rather do this in a future PR unless @vkuzo gets to it first

Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly I could also just remove the numpy dependency from dev dependencies because it's only used for a benchmark for mx that doesn't run in CI

@msaroufim msaroufim merged commit 1b78dda into main Jun 17, 2024
13 checks passed
@msaroufim msaroufim deleted the msaroufim/req branch June 17, 2024 19:14
@msaroufim msaroufim restored the msaroufim/req branch June 17, 2024 19:18
@msaroufim msaroufim deleted the msaroufim/req branch June 17, 2024 19:18
dbyoung18 pushed a commit to dbyoung18/ao that referenced this pull request Jul 31, 2024
* clean up req.txt

* yolo

* yolo

* yolo

* yolo

* yolo

* yolo

* Update dev-requirements.txt

* Delete requirements.txt

* Update utils.py

* Update README.md

* Trigger CI

* remove req from ci

* Update doc_build.yml

* remove numpy as a dependency

* yolo

* Update dev-requirements.txt

* Update setup.py
yanbing-j pushed a commit to yanbing-j/ao that referenced this pull request Dec 9, 2024
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 Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants