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.

Sign up for GitHub

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

Jump to bottom

Use new API to register custom ExecuTorch kernels into ATen #2937

Closed

larryliu0820 wants to merge 1 commit into main from export-D55907749

Contributor

larryliu0820 commented Apr 9, 2024

Summary:
Retry of D55713944
Use WRAP_TO_ATEN to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to libcustom_ops_aot_lib.so in setup.py. This is to make sure we can build libcustom_ops_aot_lib.so and install it to the correct position (<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so) and then it can be loaded by torch.ops.load_library.

Differential Revision: D55907749

pytorch-bot bot commented Apr 9, 2024 •

edited

Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/2937

📄 Preview Python docs built from this PR

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

❌ 1 New Failure

As of commit 70c9622 with merge base 6e43135 ():

NEW FAILURE - The following job has failed:

pull / unittest / macos (buck2) / macos-job (gh)
RuntimeError: Command bash /Users/ec2-user/runner/_work/_temp/exec_script failed with exit code 5

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

facebook-github-bot added the CLA Signed label

Contributor

facebook-github-bot commented Apr 9, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

facebook-github-bot added the fb-exported label

Contributor

facebook-github-bot commented Apr 9, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

larryliu0820 added a commit that referenced this pull request


          Use new API to register custom ExecuTorch kernels into ATen (#2937)

df771f3

Summary:
Pull Request resolved: #2937

Retry of D55713944
Use `WRAP_TO_ATEN` to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to `libcustom_ops_aot_lib.so` in `setup.py`. This is to make sure we can build `libcustom_ops_aot_lib.so` and install it to the correct position (`<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so`) and then it can be loaded by `torch.ops.load_library`.

Differential Revision: D55907749

larryliu0820 force-pushed the export-D55907749 branch from 30f4736 to df771f3 Compare

April 9, 2024 05:59

Contributor

facebook-github-bot commented Apr 9, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

larryliu0820 added a commit that referenced this pull request


          Use new API to register custom ExecuTorch kernels into ATen (#2937)

d3162fe

Summary:
Pull Request resolved: #2937

Retry of D55713944
Use `WRAP_TO_ATEN` to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to `libcustom_ops_aot_lib.so` in `setup.py`. This is to make sure we can build `libcustom_ops_aot_lib.so` and install it to the correct position (`<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so`) and then it can be loaded by `torch.ops.load_library`.

Differential Revision: D55907749

larryliu0820 force-pushed the export-D55907749 branch from df771f3 to d3162fe Compare

April 9, 2024 06:05

Contributor

facebook-github-bot commented Apr 9, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

larryliu0820 added a commit that referenced this pull request


          Use new API to register custom ExecuTorch kernels into ATen (#2937)

e518f9e

Summary:
Pull Request resolved: #2937

Retry of D55713944
Use `WRAP_TO_ATEN` to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to `libcustom_ops_aot_lib.so` in `setup.py`. This is to make sure we can build `libcustom_ops_aot_lib.so` and install it to the correct position (`<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so`) and then it can be loaded by `torch.ops.load_library`.

Differential Revision: D55907749

larryliu0820 force-pushed the export-D55907749 branch from d3162fe to e518f9e Compare

April 9, 2024 06:44

Contributor

facebook-github-bot commented Apr 9, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

larryliu0820 added a commit that referenced this pull request


          Use new API to register custom ExecuTorch kernels into ATen (#2937)

d9bdadc

Summary:
Pull Request resolved: #2937

Retry of D55713944
Use `WRAP_TO_ATEN` to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to `libcustom_ops_aot_lib.so` in `setup.py`. This is to make sure we can build `libcustom_ops_aot_lib.so` and install it to the correct position (`<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so`) and then it can be loaded by `torch.ops.load_library`.

Differential Revision: D55907749

larryliu0820 force-pushed the export-D55907749 branch 2 times, most recently from d9bdadc to 1a67378 Compare

April 9, 2024 07:31

larryliu0820 added a commit that referenced this pull request


          Use new API to register custom ExecuTorch kernels into ATen (#2937)

1a67378

Summary:
Pull Request resolved: #2937

Retry of D55713944
Use `WRAP_TO_ATEN` to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to `libcustom_ops_aot_lib.so` in `setup.py`. This is to make sure we can build `libcustom_ops_aot_lib.so` and install it to the correct position (`<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so`) and then it can be loaded by `torch.ops.load_library`.

Differential Revision: D55907749

Contributor

facebook-github-bot commented Apr 9, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

larryliu0820 added a commit that referenced this pull request


          Use new API to register custom ExecuTorch kernels into ATen (#2937)

5a8a804

Summary:
Pull Request resolved: #2937

Retry of D55713944
Use `WRAP_TO_ATEN` to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to `libcustom_ops_aot_lib.so` in `setup.py`. This is to make sure we can build `libcustom_ops_aot_lib.so` and install it to the correct position (`<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so`) and then it can be loaded by `torch.ops.load_library`.

Differential Revision: D55907749

larryliu0820 force-pushed the export-D55907749 branch from 1a67378 to 5a8a804 Compare

April 9, 2024 07:37

Contributor

facebook-github-bot commented Apr 9, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

lucylq approved these changes

View reviewed changes

larryliu0820 added the ciflow/trunk label

larryliu0820 added a commit that referenced this pull request


          Use new API to register custom ExecuTorch kernels into ATen (#2937)

db799c9

Summary:
Pull Request resolved: #2937

Retry of D55713944
Use `WRAP_TO_ATEN` to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to `libcustom_ops_aot_lib.so` in `setup.py`. This is to make sure we can build `libcustom_ops_aot_lib.so` and install it to the correct position (`<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so`) and then it can be loaded by `torch.ops.load_library`.

Reviewed By: lucylq

Differential Revision: D55907749

Contributor

facebook-github-bot commented Apr 11, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

larryliu0820 force-pushed the export-D55907749 branch from 5a8a804 to db799c9 Compare

April 11, 2024 05:12

Contributor

facebook-github-bot commented Apr 11, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

larryliu0820 added a commit that referenced this pull request


          Use new API to register custom ExecuTorch kernels into ATen (#2937)

64cf836

Summary:
Pull Request resolved: #2937

Retry of D55713944
Use `WRAP_TO_ATEN` to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to `libcustom_ops_aot_lib.so` in `setup.py`. This is to make sure we can build `libcustom_ops_aot_lib.so` and install it to the correct position (`<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so`) and then it can be loaded by `torch.ops.load_library`.

Reviewed By: lucylq

Differential Revision: D55907749

larryliu0820 force-pushed the export-D55907749 branch from db799c9 to 64cf836 Compare

April 11, 2024 07:44

larryliu0820 added a commit that referenced this pull request


          Use new API to register custom ExecuTorch kernels into ATen (#2937)

b1e027e

Summary:
Pull Request resolved: #2937

Retry of D55713944
Use `WRAP_TO_ATEN` to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to `libcustom_ops_aot_lib.so` in `setup.py`. This is to make sure we can build `libcustom_ops_aot_lib.so` and install it to the correct position (`<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so`) and then it can be loaded by `torch.ops.load_library`.

Reviewed By: lucylq

Differential Revision: D55907749

larryliu0820 force-pushed the export-D55907749 branch from 64cf836 to b1e027e Compare

April 11, 2024 07:49

Contributor

facebook-github-bot commented Apr 11, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

larryliu0820 added a commit that referenced this pull request


          Use new API to register custom ExecuTorch kernels into ATen (#2937)

120c145

Summary:
Pull Request resolved: #2937

Retry of D55713944
Use `WRAP_TO_ATEN` to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to `libcustom_ops_aot_lib.so` in `setup.py`. This is to make sure we can build `libcustom_ops_aot_lib.so` and install it to the correct position (`<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so`) and then it can be loaded by `torch.ops.load_library`.

Reviewed By: lucylq

Differential Revision: D55907749

larryliu0820 force-pushed the export-D55907749 branch from b1e027e to 120c145 Compare

April 11, 2024 07:54

Contributor

facebook-github-bot commented Apr 11, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

1 similar comment

Contributor

facebook-github-bot commented Apr 11, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

larryliu0820 added a commit that referenced this pull request


          Use new API to register custom ExecuTorch kernels into ATen (#2937)

fd09532

Summary:
Pull Request resolved: #2937

Retry of D55713944
Use `WRAP_TO_ATEN` to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to `libcustom_ops_aot_lib.so` in `setup.py`. This is to make sure we can build `libcustom_ops_aot_lib.so` and install it to the correct position (`<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so`) and then it can be loaded by `torch.ops.load_library`.

Reviewed By: lucylq

Differential Revision: D55907749

larryliu0820 force-pushed the export-D55907749 branch from 120c145 to fd09532 Compare

April 11, 2024 08:01

Contributor

facebook-github-bot commented Apr 11, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

larryliu0820 force-pushed the export-D55907749 branch from fd09532 to 6f9a2da Compare

April 11, 2024 08:08


          Use new API to register custom ExecuTorch kernels into ATen (#2937)

70c9622

Summary:

Retry of D55713944
Use `WRAP_TO_ATEN` to register custom ExecuTorch kernel to PyTorch.

This PR added installation logic to `libcustom_ops_aot_lib.so` in `setup.py`. This is to make sure we can build `libcustom_ops_aot_lib.so` and install it to the correct position (`<site-packages>/executorch/examples/models/llama2/custom_ops/libcustom_ops_aot_lib.so`) and then it can be loaded by `torch.ops.load_library`.

Reviewed By: lucylq

Differential Revision: D55907749

facebook-github-bot force-pushed the export-D55907749 branch from 6f9a2da to 70c9622 Compare

April 11, 2024 17:35

Contributor

facebook-github-bot commented Apr 11, 2024

This pull request was exported from Phabricator. Differential Revision: D55907749

facebook-github-bot closed this in

c322685

Contributor

facebook-github-bot commented Apr 11, 2024

This pull request has been merged in c322685.

facebook-github-bot added the Merged label

dbort reviewed

View reviewed changes

Contributor

dbort left a comment

The structure of the edits to setup.py look ok. My main worry is about the maintainability of the naming

CMakeLists.txt

		@@ -144,6 +144,8 @@ option(EXECUTORCH_BUILD_COREML "Build the Core ML backend" OFF)

		option(EXECUTORCH_BUILD_CUSTOM "Build the custom kernels" OFF)

		option(EXECUTORCH_BUILD_CUSTOM_OPS_AOT "Build the custom ops lib for AOT" OFF)

Contributor

dbort Apr 11, 2024

This name will be too vague over time: are we only going to have one library with custom ops? Or is this llama-specifc? If it is just one global library, how do we decide which kinds of custom ops get to live in it?

Contributor

kimishpatel Apr 26, 2024

yeah i agree. this custom ops were examples/models/llama specific at the time it was added. Though things have changed since.

CMakeLists.txt

		@@ -144,6 +144,8 @@ option(EXECUTORCH_BUILD_COREML "Build the Core ML backend" OFF)

		option(EXECUTORCH_BUILD_CUSTOM "Build the custom kernels" OFF)

		option(EXECUTORCH_BUILD_CUSTOM_OPS_AOT "Build the custom ops lib for AOT" OFF)

Contributor

dbort Apr 11, 2024

This name is too similar to EXECUTORCH_BUILD_CUSTOM. If you ask a random person what the different is between EXECUTORCH_BUILD_CUSTOM and EXECUTORCH_BUILD_CUSTOM_OPS_AOT they probably wouldn't have a good answer.

Really, EXECUTORCH_BUILD_CUSTOM should probably have a more descriptive name. If that's too disruptive, please update the help strings for both options to make it more clear how they differ, and help users understand which one to enable and when.

CMakeLists.txt

               if(EXECUTORCH_BUILD_CUSTOM)
                 set(EXECUTORCH_BUILD_OPTIMIZED ON)
               endif()
               if(EXECUTORCH_BUILD_CPUINFO)
                 # --- cpuinfo
+                set(ORIGINAL_CMAKE_POSITION_INDEPENDENT_CODE_FLAG

Contributor

dbort Apr 11, 2024

Since this is an internal temp var it should have a name like _original_cmake_position_independent_code_flag. Similar below.

examples/models/llama2/custom_ops/CMakeLists.txt

@@ @@ -25,7 +25,7 @@ if(NOT TORCH_ROOT) @@
                 set(TORCH_ROOT ${EXECUTORCH_ROOT}/third-party/pytorch)
               endif()
-              set(_common_compile_options -Wno-deprecated-declarations)
+              set(_common_compile_options -Wno-deprecated-declarations -fPIC)

Contributor

dbort Apr 11, 2024

Please add a comment explaining why this sets -fPIC manually instead of using CMAKE_POSITION_INDEPENDENT_CODE

Contributor Author

larryliu0820 Apr 11, 2024

I try to avoid using CMAKE_POSITION_INDEPENDENT_CODE as much as possible since it's a global variable. We have to use it for third-party libs because we can't change their CMakeLists.txt but for this one we can tweak the compile options for this specific lib.

setup.py

@@ @@ -88,6 +88,11 @@ def pybindings(cls) -> bool: @@
                   def xnnpack(cls) -> bool:
                       return cls._is_env_enabled("EXECUTORCH_BUILD_XNNPACK", default=False)
+                  @classmethod
+                  @property
+                  def llama_custom_ops(cls) -> bool:

Contributor

dbort Apr 11, 2024

Would be good to remove llama_ here since that name isn't part of the env vars.

mergennachin mentioned this pull request

disclaimer #3376

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk CLA Signed fb-exported Merged