-
Notifications
You must be signed in to change notification settings - Fork 349
require --no-build-isolation in torchao builds #3209
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/3209
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ✅ You can merge normally! (3 Unrelated Failures)As of commit 889f5da with merge base ad533f5 ( BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
34b6ec5
to
f58e799
Compare
note: |
@atalman , I'm guessing if we go forward with this we'll also need to modify https://github.com/pytorch/test-infra/blob/a287d59209eb9221fa2428c3930efdc8643ad801/.github/workflows/build_wheels_linux.yml#L245 to use --no-build-isolation, gated to torchao only? |
Warning: Unknown label
Please add the new label to .github/pytorch-probot.yml |
HI @vkuzo test-infra will execute this build: https://github.com/pytorch/test-infra/blob/a287d59209eb9221fa2428c3930efdc8643ad801/.github/workflows/build_wheels_linux.yml#L264
This is still supported |
6933c9b
to
25ce86c
Compare
Summary: TODO write me Test Plan: ```bash // both of the commands below work with-proxy USE_CPP=0 pip install -e . --no-build-isolation with-proxy USE_CPP=1 pip install -e . --no-build-isolation --verbose ``` Reviewers: Subscribers: Tasks: Tags:
25ce86c
to
889f5da
Compare
pip install $PYTORCH_DEP | ||
|
||
pip install setuptools wheel twine auditwheel | ||
pip install setuptools wheel twine auditwheel numpy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
numpy
was added to DTensor here: pytorch/pytorch#165451, so now we need to add it to the environment for successful wheel builds
CI failures are unrelated, merging |
Summary:
In recent versions of
setuptools
, the behavior of using PEP 517 (enforce build isolation, https://peps.python.org/pep-0517/) flipped from False to True. This broke torchao builds as oursetup.py
file depends on not having build isolation, as it imports build extension class definitions fromtorch
(ao/setup.py
Line 106 in 94dee9c
A long term fix would be modernize the torchao build system to honor PEP 517. However, to unbreak CI and buy us some time, in the short term we can just require build isolation for building torchao. This PR implements the short term workaround.
An AI-generated writeup I used to debug this, with more details: https://gist.github.com/vkuzo/5e6f282fda816c2cd4bce8c7fc1f30e2
Note that I had to add a
pyproject.toml
file to makepip install .
run without warnings.Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags: