Skip to content

Fix tutorial for Qualcomm AI Engine Direct Backend #2956

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
Closed
wants to merge 2 commits into from
Closed

Fix tutorial for Qualcomm AI Engine Direct Backend #2956

wants to merge 2 commits into from

Conversation

kirklandsign
Copy link
Contributor

We have refactors recently and need to update the tutorial and cmake.

See #2955 for isseues.

@kirklandsign
Fix tutorial for Qualcomm AI Engine Direct Backend
687425d 
We have refactors recently and need to update the tutorial and cmake
@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Apr 9, 2024 •
edited
Loading

🔗 Helpful Links

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

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

✅ No Failures

As of commit b152bbc with merge base b26eee8 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@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 Apr 9, 2024
@kirklandsign kirklandsign requested a review from cccclai April 9, 2024 23:58
@kirklandsign kirklandsign added the partner: qualcomm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Qualcomm label Apr 9, 2024
@facebook-github-bot
Copy link
Contributor

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

cccclai
cccclai reviewed Apr 10, 2024
View reviewed changes
backends/qualcomm/CMakeLists.txt
@@ -253,7 +253,7 @@ target_link_libraries(qnn_executorch_backend
qnn_executorch_header
qnn_schema
qnn_manager
executorch
executorch_no_prim_ops
Copy link
Contributor

Choose a reason for hiding this comment

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

how do we link it with prim ops lib?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In

executorch/examples/qualcomm/CMakeLists.txt

Lines 102 to 108 in 687425d

target_link_libraries(qnn_executor_runner
qnn_executorch_backend
full_portable_ops_lib
etdump
${FLATCCRT_LIB}
gflags
)
, maybe full_portable_ops_lib has it… Otherwise we need to add executorch explicitly

For other users, add executorch explicitly to binary helps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TLDR is for users who use qnn_executorch_backend as a dependency, it should also have executorch as a dependency now.

cccclai
cccclai reviewed Apr 10, 2024
View reviewed changes
docs/source/build-run-qualcomm-ai-engine-direct-backend.md Outdated
Comment on lines 141 to 142
mkdir cmake_android_out
cd cmake_android_out
Copy link
Contributor

Choose a reason for hiding this comment

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

why this change is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can use old one. simply it's already in gitignore.

Copy link
Contributor

Choose a reason for hiding this comment

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

let's use the old one probably? Try to make the change minimum

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@facebook-github-bot
Copy link
Contributor

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

@kirklandsign kirklandsign mentioned this pull request Apr 10, 2024
Closed
@cccclai
Copy link
Contributor

cccclai commented Apr 10, 2024

@chiwwang can you help with tagging reviewers?

@chiwwang
Copy link
Contributor

Hey @chuntl , may I know if you're able to give this a try?

@chuntl
Copy link
Contributor

chuntl commented Apr 10, 2024

Rebase to mainline and do the "./install_requirements.sh“ meet the target missing issue:

image

It seems there're some WIP PRs to fix the CI issue and affect to test this PR.

@chiwwang
Copy link
Contributor

Rebase to mainline and do the "./install_requirements.sh“ meet the target missing issue:

image

It seems there're some WIP PRs to fix the CI issue and affect to test this PR.

Got it.
Assuming this error will be fixed, if the content of this PR looks good?

@chuntl
Copy link
Contributor

chuntl commented Apr 10, 2024 •
edited
Loading

Rebase to mainline and do the "./install_requirements.sh“ meet the target missing issue:
image
It seems there're some WIP PRs to fix the CI issue and affect to test this PR.

Got it. Assuming this error will be fixed, if the content of this PR looks good?

Yes, It looks good to me. But it's better to double-check after the error fixed.

chiwwang
chiwwang reviewed Apr 10, 2024
View reviewed changes
docs/source/build-run-qualcomm-ai-engine-direct-backend.md
-DCMAKE_INSTALL_PREFIX=$PWD \
-DEXECUTORCH_BUILD_SDK=ON \
-DEXECUTORCH_BUILD_QNN=ON \
Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly need something like CMAKE_INSTALL_PREFIX if we don't want to require root priviledge.
mmmm... would it be a good idea to leave a reminder here?

@kirklandsign
Copy link
Contributor Author

Rebase to mainline and do the "./install_requirements.sh“ meet the target missing issue:

image

It seems there're some WIP PRs to fix the CI issue and affect to test this PR.

Since we updated https://github.com/pytorch/executorch/blob/main/build/cmake_deps.toml, we need to rm -rf pip-out. We will note this in the doc.

@chuntl
Copy link
Contributor

chuntl commented Apr 11, 2024 •
edited
Loading

This PR looks good to me, thank you!

@facebook-github-bot
Copy link
Contributor

@kirklandsign merged this pull request in c7fd394.

kirklandsign added a commit to kirklandsign/executorch that referenced this pull request Apr 11, 2024
@kirklandsign
Fix tutorial for Qualcomm AI Engine Direct Backend (pytorch#2956)
1636d89 
Summary:
We have refactors recently and need to update the tutorial and cmake.

See pytorch#2955 for isseues.

Pull Request resolved: pytorch#2956

Reviewed By: mcr229, cccclai

Differential Revision: D55947725

Pulled By: kirklandsign

fbshipit-source-id: f23af28b9a8fe071223d8ffa922a6cd4e49efe61
(cherry picked from commit c7fd394)
@kirklandsign kirklandsign mentioned this pull request Apr 11, 2024
Closed
kirklandsign added a commit to kirklandsign/executorch that referenced this pull request Apr 12, 2024
@kirklandsign
Fix tutorial for Qualcomm AI Engine Direct Backend (pytorch#2956)
9b8e74e 
Summary:
We have refactors recently and need to update the tutorial and cmake.

See pytorch#2955 for isseues.

Pull Request resolved: pytorch#2956

Reviewed By: mcr229, cccclai

Differential Revision: D55947725

Pulled By: kirklandsign

fbshipit-source-id: f23af28b9a8fe071223d8ffa922a6cd4e49efe61
(cherry picked from commit c7fd394)
kirklandsign added a commit to kirklandsign/executorch that referenced this pull request Apr 12, 2024
@kirklandsign
Fix tutorial for Qualcomm AI Engine Direct Backend (pytorch#2956)
09f8afe 
Summary:
We have refactors recently and need to update the tutorial and cmake.

See pytorch#2955 for isseues.

Pull Request resolved: pytorch#2956

Reviewed By: mcr229, cccclai

Differential Revision: D55947725

Pulled By: kirklandsign

fbshipit-source-id: f23af28b9a8fe071223d8ffa922a6cd4e49efe61
(cherry picked from commit c7fd394)
@kirklandsign kirklandsign mentioned this pull request Apr 12, 2024
Merged
guangy10 pushed a commit that referenced this pull request Apr 17, 2024
@kirklandsign
Fix tutorial for Qualcomm AI Engine Direct Backend (#2956) (#3026)
212e91f 
Summary:
We have refactors recently and need to update the tutorial and cmake.

See #2955 for isseues.

Pull Request resolved: #2956

Reviewed By: mcr229, cccclai

Differential Revision: D55947725

Pulled By: kirklandsign

fbshipit-source-id: f23af28b9a8fe071223d8ffa922a6cd4e49efe61
(cherry picked from commit c7fd394)
This was referenced Apr 25, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chiwwang chiwwang chiwwang left review comments

@cccclai cccclai cccclai approved these changes

@chuntl chuntl chuntl approved these changes

Assignees

@cccclai cccclai

Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged partner: qualcomm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Qualcomm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kirklandsign @facebook-github-bot @cccclai @chiwwang @chuntl