Qualcomm AI Engine Direct - Support topk #5870

winskuo-quic · 2024-10-04T05:49:52Z

Summary

Support topK
Properly decompose einsum for quantization annotation to work properly
Unify warning messages in op builder
Add UT

pytorch-bot · 2024-10-04T05:49:56Z

🔗 Helpful Links

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

📄 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! (1 Unrelated Failure)

As of commit c2712dd with merge base 2e4e17c ():

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

pull / unittest-arm (buck2) / linux-job (gh) (trunk failure)
../../dev/ops/test_add.py::TestSimpleAdd::test_add2_tosa_BI_4

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

winskuo-quic · 2024-10-04T05:51:12Z

Hi @cccclai,
This PR is to enable topK op builder and properly decompose einsum for quantization.
Please have a look.
Thanks.

cccclai

This looks great! In the long term, it will be great to seperate changes such that one PR is for one feature. This PR seems including other stuff more than topk

cccclai · 2024-10-05T22:40:20Z

backends/qualcomm/passes/decompose_einsum.py

+from torch.fx.experimental.proxy_tensor import make_fx
+
+
+class DecomposeEinsum(ExportPass):


This pattern seems common enough and we will see if we support this kind of logic more easily.

cccclai · 2024-10-05T22:41:10Z

backends/qualcomm/tests/models.py

@@ -416,6 +416,17 @@ def forward(self, x):
        return torch.sum(self.first(x), dim=(2, 3), keepdim=False)


+class Conv2dTopK(torch.nn.Module):


I assume it's just to test this kind of common pattern, and they won't be ussed?

Yes. This is just for testing purpose to check layout transform is working properly.

cccclai · 2024-10-05T22:41:43Z

backends/qualcomm/tests/models.py

+        super().__init__()
+
+    def forward(self, i, j):
+        return torch.einsum("i,j->ij", i, j)


Is einsum used somewhere?

There is einsum operation in MOEFeedForward under executorch.examples.models.llama2.llama_transformer. Even though we are not using MOEFeedForward now, but we think it is a good test case to ensure our topK is working as expected.

Ah I see, so actually the einsum is probably not the best for qnn backend, it's better to replace with other way (I try it before) to get a better performance. But it's still good to run einsum.

facebook-github-bot · 2024-10-05T22:43:42Z

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

winskuo-quic · 2024-10-07T00:52:01Z

This looks great! In the long term, it will be great to seperate changes such that one PR is for one feature. This PR seems including other stuff more than topK

Thanks for all the suggestions, and I will separate individual features into smaller PRs in the future. Sorry that the title is a little misleading. Originally, we were trying to enable topK and used MOEFeedForward as a test case since there is topK in there. However, we ran into other issues like einsum decomposition, etc. I will keep the title more accurate in the future and make sure each PR is focusing on one feature.

Thanks!

facebook-github-bot · 2024-10-08T03:55:36Z

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

cccclai · 2024-10-08T04:56:01Z

Hey I may need to get #5925 checked in first because it needs to get cherry pick. Then I'll merge this one.

winskuo-quic · 2024-10-08T05:16:02Z

Hey I may need to get #5925 checked in first because it needs to get cherry pick. Then I'll merge this one.

Sounds good! I can also rebase after #5925 is merged if there are any conflicts.

cccclai · 2024-10-09T03:26:05Z

Hey I may need to get #5925 checked in first because it needs to get cherry pick. Then I'll merge this one.

Sounds good! I can also rebase after #5925 is merged if there are any conflicts.

Hi sorry for keeping you waiting, another PR #6025 needs to be merged and also needed for beta...the beta release is happening in 2 days and hopefully that is the last PR

cccclai · 2024-10-10T04:16:25Z

Hi it's ready to merge now, can you rebase? Thank you

winskuo-quic · 2024-10-11T09:30:51Z

Hi it's ready to merge now, can you rebase? Thank you

I have just rebased. Thanks

facebook-github-bot · 2024-10-11T16:33:08Z

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

facebook-github-bot · 2024-10-11T17:53:33Z

@cccclai merged this pull request in d094b09.

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 Oct 4, 2024

cccclai approved these changes Oct 5, 2024

View reviewed changes

Qualcomm AI Engine Direct - Support topk

c2712dd

winskuo-quic force-pushed the dev1/winskuo/topk branch from f8dd1b8 to c2712dd Compare October 11, 2024 09:29

facebook-github-bot closed this in d094b09 Oct 11, 2024

facebook-github-bot added the Merged label Oct 11, 2024

		from torch.fx.experimental.proxy_tensor import make_fx


		class DecomposeEinsum(ExportPass):

		@@ -416,6 +416,17 @@ def forward(self, x):
		return torch.sum(self.first(x), dim=(2, 3), keepdim=False)


		class Conv2dTopK(torch.nn.Module):

Qualcomm AI Engine Direct - Support topk #5870

Qualcomm AI Engine Direct - Support topk #5870

Uh oh!

Conversation

winskuo-quic commented Oct 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

pytorch-bot bot commented Oct 4, 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/5870

✅ You can merge normally! (1 Unrelated Failure)

Uh oh!

winskuo-quic commented Oct 4, 2024

Uh oh!

cccclai left a comment

Choose a reason for hiding this comment

Uh oh!

cccclai Oct 5, 2024

Choose a reason for hiding this comment

Uh oh!

cccclai Oct 5, 2024

Choose a reason for hiding this comment

Uh oh!

winskuo-quic Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

cccclai Oct 5, 2024

Choose a reason for hiding this comment

Uh oh!

winskuo-quic Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cccclai Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Oct 5, 2024

Uh oh!

winskuo-quic commented Oct 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Oct 8, 2024

Uh oh!

cccclai commented Oct 8, 2024

Uh oh!

winskuo-quic commented Oct 8, 2024

Uh oh!

cccclai commented Oct 9, 2024

Uh oh!

cccclai commented Oct 10, 2024

Uh oh!

winskuo-quic commented Oct 11, 2024

Uh oh!

facebook-github-bot commented Oct 11, 2024

Uh oh!

facebook-github-bot commented Oct 11, 2024

Uh oh!

Uh oh!

winskuo-quic commented Oct 4, 2024 •

edited

Loading

pytorch-bot bot commented Oct 4, 2024 •

edited

Loading

winskuo-quic Oct 7, 2024 •

edited

Loading

winskuo-quic commented Oct 7, 2024 •

edited

Loading