Add qnn backend to pip packages #13794
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/13794
Note: Links to docs will display an error until the docs builds have been completed. ❌ 3 New Failures, 37 Pending, 4 Unrelated FailuresAs of commit 396b21d with merge base 8496f27 ( NEW FAILURES - The following jobs have failed:
FLAKY - The following job failed but was likely due to flakiness present on trunk:
BROKEN TRUNK - The following jobs failed but was 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. |
meta-cla
bot
added
the
CLA Signed
This PR needs a
|
cccclai
force-pushed
the
qnn_pip
branch
2 times, most recently
from
9014e21 to
0ca5bd3
Compare
|
working version ded7cf0 |
| TEMP_ENV_DIR=$(mktemp -d) | ||
| echo "Using temporary directory: $TEMP_ENV_DIR" | ||
|
|
||
| conda create -y -p "$TEMP_ENV_DIR/env" python=3.10 |
There was a problem hiding this comment.
should we also check for other python versions? Looks like we do both 3.10 and 3.12 https://pypi.org/project/executorch/0.7.0/#files, so maybe test both?
There was a problem hiding this comment.
Also if venv is used, can we check it works with venv as well?
There was a problem hiding this comment.
Added venv in the test too
| conda run -p "$TEMP_ENV_DIR/env" python -c "import executorch; print('executorch imported successfully')" | ||
| conda run -p "$TEMP_ENV_DIR/env" python -c "import executorch.backends.qualcomm; print('executorch.backends.qualcomm imported successfully')" |
There was a problem hiding this comment.
is it worth checking if the so files are downloaded and installed? Or the script running would suffice
There was a problem hiding this comment.
The script includes the lowering steps, should be enough if lowering is successful
| - Only runs on Linux x86 platforms. Skips otherwise. | ||
| """ | ||
| print("Downloading Qualcomm SDK...") | ||
| QNN_VERSION = "2.37.0.250724" |
There was a problem hiding this comment.
nit: maybe refactor to get/set qnn version functions? Mainly for ease of maintenance.
There was a problem hiding this comment.
Yeah I'd need to have a separate PR to remove the hardcoded numbers #13960
| libcxx = target_dir / "libc++.so.1.0" | ||
| libcxx_abi = target_dir / "libc++abi.so.1.0" | ||
| if libcxx.exists(): | ||
| os.symlink("libc++.so.1.0", target_dir / "libc++.so.1") |
There was a problem hiding this comment.
this is creating symlink to what?
There was a problem hiding this comment.
a few ones like libc++.so.1.0 to libc++.so.1, that's extra burden for users imo
There was a problem hiding this comment.
these are system libs generally. I dont know what the expectation is for qnn sdk. Can you tag someone from qcomm to at least understand if there is specific version of libc++ that is need for specific version of qnn sdk.
My biggest concern with all these is that different libraries may have different libc++ requirement. What if mtk's sdk doesnt work with this specific libc++.
| if self._ran_build: | ||
| return | ||
|
|
||
| try: |
There was a problem hiding this comment.
Can this be a separate build_ext? is it possible to do?
There was a problem hiding this comment.
Each setup() only has one build_ext to override, but I can double check if we can move it to build instead (where our current main build happens)
|
mark as draft to address some feedback |
cccclai
requested review from
haowhsu-quic,
shewu-quic,
winskuo-quic and
DannyYuyang-quic
|
|
||
| # Run build.sh with SDK path exported | ||
| env = dict(**os.environ) | ||
| env["QNN_SDK_ROOT"] = str(sdk_path) |
There was a problem hiding this comment.
If I understand correctly, this env variable only visible when doing pip install. If user starts a new process, will they be notified to use the correct one?
There was a problem hiding this comment.
Ah never mind, this is for building delegate related libraries for packaging.
There was a problem hiding this comment.
Yes, only building delegates for the wheel package
| env_flag = os.getenv("EXECUTORCH_BUILDING_WHEEL", "0").lower() | ||
| # If users have preinstalled QNN_SDK_ROOT, we will use it. | ||
| qnn_sdk_root_flag = os.getenv("QNN_SDK_ROOT", None) | ||
| if env_flag not in ("1", "true", "yes") and not qnn_sdk_root_flag: |
There was a problem hiding this comment.
I wonder if the same user will keep downloading SDK if QNN_SDK_ROOT is never set?
There was a problem hiding this comment.
No, if QNN_SDK_ROOT is not set, we will use the predefined path SDK_DIR = PKG_ROOT / "sdk" / "qnn". Inside the download_qnn_sdk.py logic, we will check if the library is there already, if yes, we will just use those files
|
@haowhsu-quic I will merge this PR for now, but would like to hear your feedback/concern and address them |
This reverts commit 068d341. It broke Windows wheels per https://hud.pytorch.org/hud/pytorch/executorch/main/1?per_page=50&name_filter=windows ghstack-source-id: da82656 ghstack-comment-id: 3281366424 Pull-Request: #14212
This reverts commit 068d341. It broke Windows wheels per https://hud.pytorch.org/hud/pytorch/executorch/main/1?per_page=50&name_filter=windows
| from .scripts.download_qnn_sdk import install_qnn_sdk | ||
|
|
||
|
|
||
| env_flag = os.getenv("EXECUTORCH_BUILDING_WHEEL", "0").lower() |
There was a problem hiding this comment.
Gate on another flat like BUILDING_WHEEL_FOR_LINUX or something similar
| ) | ||
| QAIRT_CONTENT_DIR = f"qairt/{QNN_VERSION}" | ||
|
|
||
| if not is_linux_x86(): |
There was a problem hiding this comment.
I have the linux check here @kimishpatel
| if env_flag not in ("1", "true", "yes") and not qnn_sdk_root_flag: | ||
| ok = install_qnn_sdk() | ||
|
|
||
| if not ok: |
There was a problem hiding this comment.
On windows this will always be not ok. So you want to raise this error only if we are linux.
There was a problem hiding this comment.
oh I see what you meant, okay let me add it
|
|
||
| os.environ["EXECUTORCH_BUILDING_WHEEL"] = "1" | ||
|
|
||
| with tempfile.TemporaryDirectory() as tmpdir: |
There was a problem hiding this comment.
Also this componenents should be trigerred only for linux as well
Add QNN to pip package, target linux only. The qnn sdk will be installed when user import from any submodule in executorch.backend.qualcomm the first time How to test: 1. When building the wheel package ``` conda create -yn executorch python=3.10.0 && conda activate executorch ./install_executorch.sh EXECUTORCH_BUILDING_WHEEL=1 python setup.py bdist_wheel ``` The wheel package will be under `dist` folder 2. When users install the wheel packages Start a new terminal from a fresh repro (like /tmp) ``` unset EXECUTORCH_BUILDING_WHEEL // to make sure this env is not set pip install executorch-0.8.0a0+8fe177e-cp310-cp310-linux_x86_64.whl // path to the wheel package pip install install torch=="2.9.0.dev20250801" --index-url "https://download.pytorch.org/whl/nightly/cpu" pip install --pre torchao --index-url "https://download.pytorch.org/whl/nightly/cpu" ``` Run the script the same as `test_wheel_package_qnn.sh`
This reverts commit 068d341. It broke Windows wheels per https://hud.pytorch.org/hud/pytorch/executorch/main/1?per_page=50&name_filter=windows
Add QNN to pip package, target linux only. The qnn sdk will be installed when user import from any submodule in executorch.backend.qualcomm the first time
How to test:
The wheel package will be under
distfolderStart a new terminal from a fresh repro (like /tmp)
Run the script the same as
test_wheel_package_qnn.sh