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

[1/2] Wean off of PYBIND in favor of torch.ops.load_library #1275

Closed
wants to merge 4 commits into from

Conversation

janeyx99
Copy link
Contributor

@janeyx99 janeyx99 commented Nov 12, 2024

Pave the path for python agnostic ao by removing depending on PYBIND (which is not python agnostic).

Concretely, what happened this PR?

  • no more PYBIND, so no more init.cpp
  • for all non-CUDA platforms, ao no longer has custom cpp extensions, and thus ao is a pure python lib
  • so we skip auditwheel (which is used for when the platform is linux) for all non-cuda wheel builds. (This does also mean we now needlessly build the same python wheel for every other platform rocm, cpu, xpu...)
  • no more PYBIND also means no more torchao._C, which we're replacing with a load_library call

This PR should have no failures. The next PR will be targeting the wheel process to only output one wheel for every python version.

Stack from ghstack (oldest at bottom):

Copy link

pytorch-bot bot commented Nov 12, 2024

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit 2ea7b4c with merge base f96e5ec (image):

NEW FAILURE - The following job has failed:

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

janeyx99 added a commit that referenced this pull request Nov 12, 2024
ghstack-source-id: 581221e576fde7f1744477c06027278fed229b89
Pull Request resolved: #1275
@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 Nov 12, 2024
@janeyx99 janeyx99 added the topic: for developers Use this tag if this PR is mainly developer facing label Nov 12, 2024
@janeyx99 janeyx99 changed the title Wean ao off of PYBIND [part 1] Wean ao off of PYBIND in favor of torch.ops.load_library Nov 12, 2024
Pave the path for python agnostic ao by removing depending on PYBIND (which is not python agnostic).

This PR should have no failures. The next PR will be targeting the wheel process to only output one wheel for every python version.




[ghstack-poisoned]
janeyx99 added a commit that referenced this pull request Nov 12, 2024
ghstack-source-id: 16299c82626a4673d4130e0b57c51c4dfdc79ab7
Pull Request resolved: #1275
Pave the path for python agnostic ao by removing depending on PYBIND (which is not python agnostic).

Concretely, what happened this PR?
- no more PYBIND, so no more init.cpp
- for all non-CUDA platforms, ao no longer has custom cpp extensions, and thus ao is a pure python lib
- so we skip auditwheel (which is used for when the platform is linux) for all non-cuda wheel builds
- no more PYBIND also means no more torchao._C, which we're replacing with a load_library call

This PR should have no failures. The next PR will be targeting the wheel process to only output one wheel for every python version.




[ghstack-poisoned]
janeyx99 added a commit that referenced this pull request Nov 12, 2024
ghstack-source-id: 04eb397def723b4cd24e1df9d0bb791988eadfaa
Pull Request resolved: #1275
@janeyx99 janeyx99 changed the title Wean ao off of PYBIND in favor of torch.ops.load_library Wean off of PYBIND in favor of torch.ops.load_library Nov 12, 2024
Pave the path for python agnostic ao by removing depending on PYBIND (which is not python agnostic).

Concretely, what happened this PR?
- no more PYBIND, so no more init.cpp
- for all non-CUDA platforms, ao no longer has custom cpp extensions, and thus ao is a pure python lib
- so we skip auditwheel (which is used for when the platform is linux) for all non-cuda wheel builds
- no more PYBIND also means no more torchao._C, which we're replacing with a load_library call

This PR should have no failures. The next PR will be targeting the wheel process to only output one wheel for every python version.




[ghstack-poisoned]
janeyx99 added a commit that referenced this pull request Nov 12, 2024
ghstack-source-id: f08db0ecc13a1e1aa40ab87d24fba641844b84e5
Pull Request resolved: #1275
@@ -110,6 +110,9 @@ def get_extensions():
if use_cuda:
sources += cuda_sources

if len(sources) == 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

if not sources sounds more pythonic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ehhhh it looks less readable to me

@janeyx99 janeyx99 changed the title Wean off of PYBIND in favor of torch.ops.load_library Wean off of PYBIND in favor of torch.ops.load_library (pt1) Nov 13, 2024
@janeyx99 janeyx99 changed the title Wean off of PYBIND in favor of torch.ops.load_library (pt1) [1/2] Wean off of PYBIND in favor of torch.ops.load_library Nov 13, 2024
@janeyx99
Copy link
Contributor Author

Closing in favor of #1276 as that one passes all CI. It looks like Build M1 Wheels does not work with ghstack

@janeyx99 janeyx99 closed this Nov 13, 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. topic: for developers Use this tag if this PR is mainly developer facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants