Skip to content

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

Closed
janeyx99 wants to merge 4 commits into
gh/janeyx99/1/basefrom
gh/janeyx99/1/head
Closed

[1/2] Wean off of PYBIND in favor of torch.ops.load_library#1275
janeyx99 wants to merge 4 commits into
gh/janeyx99/1/basefrom
gh/janeyx99/1/head

Conversation

@janeyx99

@janeyx99 janeyx99 commented Nov 12, 2024

Copy link
Copy Markdown
Contributor

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):

@pytorch-bot

pytorch-bot Bot commented Nov 12, 2024

Copy link
Copy Markdown

🔗 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: 581221e
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: 16299c8
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: 04eb397
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: f08db0e
Pull Request resolved: #1275
Comment thread setup.py
if use_cuda:
sources += cuda_sources

if len(sources) == 0:

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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