Fix wheel build and smoke test #4429

kirklandsign · 2024-07-25T23:03:21Z

Test Plan:

Label ciflow/binaries to build wheel
CI and OSS CI. This makes sure that smoke test can pass, and a portable model can export and run E2E. Python 3.9 is not supported for now.
Run these helps validate unittest failure for quantized ops

pip uninstall executorch -y
rm dist/*.whl
source build/packaging/env_var_script_m1.sh # or Linux
python setup.py bdist_wheel
pip install dist/*.whl

Then in Python, try

from executorch.extension.pybindings import portable_lib
from executorch.exir.dialects._ops import ops
import executorch.kernels.quantized
ops.edge.quantized_decomposed.add.default.to_out_variant().name()

pytorch-bot · 2024-07-25T23:03:23Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

✅ You can merge normally! (2 Unrelated Failures)

As of commit 371b022 with merge base 76f0b61 ():

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

Build Linux Wheels / pytorch/executorch / upload / manywheel-py3_9-cpu (gh) (#4308)
Unable to find an artifact with the name: pytorch_executorch__3.9_cpu_x86_64
Build M1 Wheels / pytorch/executorch / upload / wheel-py3_9-cpu (gh) (#4309)
Unable to find an artifact with the name: pytorch_executorch__3.9_cpu_

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

kirklandsign · 2024-07-25T23:38:12Z

Also need to fix rpath for custom_ops

facebook-github-bot · 2024-08-02T21:56:56Z

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

build/packaging/post_build_script.sh

CMakeLists.txt

setup.py

facebook-github-bot · 2024-08-02T23:02:54Z

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

dbort

This is looking so much better! Thank you for figuring out the dependency stuff.

dbort · 2024-08-02T23:10:03Z

CMakeLists.txt

@@ -755,6 +753,16 @@ if(EXECUTORCH_BUILD_PYBIND)
                 # the torch libs are in `site-packages/torch/lib`.
                 BUILD_RPATH "@loader_path/../../../torch/lib"
                 INSTALL_RPATH "@loader_path/../../../torch/lib"
+                 BUILD_RPATH "@loader_path/../../"


Please add a comment explaining these two new RPATH lines, since the "site-packages/torch-lib" comment doesn't apply to them.

dbort · 2024-08-02T23:10:55Z

CMakeLists.txt

+      portable_lib
+      PROPERTIES # Assume that this library will be installed in
+                 # `site-packages/executorch/extension/pybindings`, and that
+                 # the custom_ops_aot_lib should be found with relative path.


Does this mean that custom_ops_aot_lib.so will be installed in the root of the executorch pip package? I'd rather install it in the same directory as portable_lib, or under something like /data/lib.

it should be under site-packages/executorch/extension/llm/custom_ops/

dbort · 2024-08-02T23:14:46Z

setup.py

+                "executorch",
+            )
+        )
+        ext_modules.append(
+            # Install the prebuilt library for quantized ops required by custom ops.
+            BuiltFile(
+                "kernels/quantized/libquantized_ops_aot_lib.*",
+                "executorch",


The second arg to BuiltFile is the name of the destination file or directory. If the intent is to install to a directory, then the destination should end in a / character. But please see if "executorch/data/lib/" works: that would address my comment above about not installing these in the root of the pip package, though you'd also need to update the RPATH. Or you could try setting the dest to executorch/extension/pybindings/ so that the libs end up in the same directory as _portable_lib.so.

Your script works! Yes the trailing / makes a difference

facebook-github-bot · 2024-08-02T23:54:28Z

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-08-06T23:19:15Z

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

larryliu0820 · 2024-08-06T23:23:11Z

extension/pybindings/portable_lib.py

+ try:
+     from pathlib import Path
+     libs = list(Path(__file__).parent.parent.parent.resolve().glob("**/libquantized_ops_aot_lib.*"))
+     del Path
+     assert len(libs) == 1, f"Expected 1 library but got {len(libs)}"
+     _torch.ops.load_library(libs[0])
+ except:
+     pass
+


So we probably don't want this code to live here. We should load it whenever we need it. I think we should add this code in init.py under this directory, so that you can import it in the unittest here

facebook-github-bot · 2024-08-06T23:59:41Z

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-08-07T00:27:45Z

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

larryliu0820 · 2024-08-07T00:46:07Z

kernels/quantized/__init__.py

+
+    libs = list(
+        Path(__file__)
+        .parent.parent.parent.resolve()


Ideally the so file should just under the same directory.

I see. I copied to the same directory so Path(file).parent should be good.

facebook-github-bot · 2024-08-07T00:56:35Z

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

dbort

The latest changes look good.

Please update the PR description with a Test Plan: section explaining how you have been testing all of this. It will be very useful to have the next time we need to mess with wheel issues.

facebook-github-bot · 2024-08-07T01:40:06Z

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-08-07T02:05:54Z

@kirklandsign has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-08-07T05:44:51Z

@kirklandsign merged this pull request in bf477e4.

Summary: Pull Request resolved: pytorch#4429 Test Plan: 1. Label `ciflow/binaries` to build wheel 2. CI and OSS CI. This makes sure that smoke test can pass, and a portable model can export and run E2E. Python 3.9 is not supported for now. 3. Run these helps validate unittest failure for quantized ops ``` pip uninstall executorch -y rm dist/*.whl source build/packaging/env_var_script_m1.sh # or Linux python setup.py bdist_wheel pip install dist/*.whl ``` Then in Python, try ``` from executorch.extension.pybindings import portable_lib from executorch.exir.dialects._ops import ops import executorch.kernels.quantized ops.edge.quantized_decomposed.add.default.to_out_variant().name() ``` Reviewed By: larryliu0820 Differential Revision: D60694247 Pulled By: kirklandsign fbshipit-source-id: 4da77493c0e7a868f86b583c9c33d54d08c3e48b

Fix wheel build and smoke test

86eaab8

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 Jul 25, 2024

kirklandsign added the ciflow/binaries label Jul 25, 2024

Need quantized lib as well

1d197d6

kirklandsign mentioned this pull request Aug 1, 2024

Use rpath for pypi artifact #4492

Closed

kirklandsign added 5 commits August 1, 2024 11:54

Fix

68b5ad0

Add quantized

32ca7a9

Fix build

412fae6

Use custom_ops dir

5c379d6

Fixed macos

638988f

clean up

2aa9d29

dbort requested changes Aug 2, 2024

View reviewed changes

Fix comments

6ad6514

rebase stuff

15518b7

dbort requested changes Aug 2, 2024

View reviewed changes

Update install dest

5b9d3da

kirklandsign added 3 commits August 2, 2024 17:04

Should still use <executorch>/kernels/quantized

94d54b9

add missing slash

cfc68e4

Load quantized_ops_aot_lib during portable load

21c9673

larryliu0820 reviewed Aug 6, 2024

View reviewed changes

kirklandsign added 3 commits August 6, 2024 16:38

Move import to quantized

e7fad82

Copy kernels/quantized to package

2417267

Linter

e482f74

Linter

a7ff4ea

larryliu0820 reviewed Aug 7, 2024

View reviewed changes

kirklandsign added 2 commits August 6, 2024 17:53

Add TARGETS rules

212d023

Fix quantized path

663e909

larryliu0820 approved these changes Aug 7, 2024

View reviewed changes

dbort approved these changes Aug 7, 2024

View reviewed changes

Linter

28d4571

BUCK visibility

371b022

facebook-github-bot closed this in bf477e4 Aug 7, 2024

facebook-github-bot added the Merged label Aug 7, 2024

kirklandsign deleted the fix-wheel-smoke-test branch August 8, 2024 03:43

Fix wheel build and smoke test #4429

Fix wheel build and smoke test #4429

Uh oh!

Conversation

kirklandsign commented Jul 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Jul 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ You can merge normally! (2 Unrelated Failures)

Uh oh!

kirklandsign commented Jul 25, 2024

Uh oh!

facebook-github-bot commented Aug 2, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

facebook-github-bot commented Aug 2, 2024

Uh oh!

dbort left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 2, 2024

Uh oh!

facebook-github-bot commented Aug 6, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 6, 2024

Uh oh!

facebook-github-bot commented Aug 7, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 7, 2024

Uh oh!

dbort left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 7, 2024

Uh oh!

facebook-github-bot commented Aug 7, 2024

Uh oh!

facebook-github-bot commented Aug 7, 2024

Uh oh!

Uh oh!

kirklandsign commented Jul 25, 2024 •

edited

Loading

pytorch-bot bot commented Jul 25, 2024 •

edited

Loading