Skip to content

Qualcomm AI Engine Direct - Phase out QCIR flow since it is no longer… #12583

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Qualcomm AI Engine Direct - Phase out QCIR flow since it is no longer… #12583

wants to merge 1 commit into from
+7 −797

Conversation

DannyYuyang-quic
Copy link
Collaborator

@DannyYuyang-quic DannyYuyang-quic commented Jul 17, 2025 •
edited
Loading

Summary

  • Remove all QCIR-related files and the associated custom protocol.

Test plan

General CI

cc: @haowhsu-quic, @cccclai

@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Jul 17, 2025 •
edited
Loading

🔗 Helpful Links

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

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 f2aa739 with merge base 45846c8 (image):

BROKEN TRUNK - The following job failed but were 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 meta-cla 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 17, 2025
@DannyYuyang-quic
Copy link
Collaborator Author

@pytorchbot label "release notes: qualcomm"

@pytorch-bot pytorch-bot bot added the release notes: qualcomm Changes to the Qualcomm backend delegate label Jul 17, 2025
@DannyYuyang-quic
Copy link
Collaborator Author

DannyYuyang-quic commented Jul 17, 2025 •
edited
Loading

Hi @cccclai,
We phase out all QCIR-related protocols as they are no longer in use.
Please have a look.
Thanks!!

@haowhsu-quic haowhsu-quic mentioned this pull request Jul 17, 2025
Open
@DannyYuyang-quic DannyYuyang-quic force-pushed the dev1/danny/deprecate_qcir_and_CustomProtocol branch from edee173 to 42000f1 Compare July 17, 2025 13:33
@cccclai
Copy link
Contributor

cccclai commented Jul 17, 2025

Thanks! Trying to refresh my memory, is QCIR only used by online prepare, and now online prepare switch to QNN IR (and it only supports qnn 2.35), correct?

@cccclai
Copy link
Contributor

cccclai commented Jul 17, 2025

Also seems like lots of changes with recent PR...can we prioritize checking a stories.pte file as part of CI so we can know if there is any BC breaking changes

@haowhsu-quic
Copy link
Collaborator

Thanks! Trying to refresh my memory, is QCIR only used by online prepare, and now online prepare switch to QNN IR (and it only supports qnn 2.35), correct?

Yes, and we used to leverage QCIR on multi-method compiling as well. I think the QNN IR support (DLC) starts from 2.29.

@DannyYuyang-quic
Copy link
Collaborator Author

DannyYuyang-quic commented Jul 18, 2025 •
edited
Loading

Also seems like lots of changes with recent PR...can we prioritize checking a stories.pte file as part of CI so we can know if there is any BC breaking changes

We currently have compile_only and pre_gen_pte flags in TestExampleLLMScript to help check for BC
such as compiling with the n-1 version and running on the current (n) version. It would be great to align on where the artifact(pte path) should live.

@cccclai
Copy link
Contributor

cccclai commented Jul 18, 2025

Also seems like lots of changes with recent PR...can we prioritize checking a stories.pte file as part of CI so we can know if there is any BC breaking changes

We currently have compile_only and pre_gen_pte flags in TestExampleLLMScript to help check for BC such as compiling with the n-1 version and running on the current (n) version. It would be great to align on where the artifact(pte path) should live.

I think we can put them in a unified folder under qualcomm/backends for the qnn schema related BC checking, and have one .pte under examples/qualcomm/oss_scripts/llama for checking llama BC changes.

@DannyYuyang-quic
Copy link
Collaborator Author

DannyYuyang-quic commented Jul 21, 2025 •
edited
Loading

Also seems like lots of changes with recent PR...can we prioritize checking a stories.pte file as part of CI so we can know if there is any BC breaking changes

We currently have compile_only and pre_gen_pte flags in TestExampleLLMScript to help check for BC such as compiling with the n-1 version and running on the current (n) version. It would be great to align on where the artifact(pte path) should live.

I think we can put them in a unified folder under qualcomm/backends for the qnn schema related BC checking, and have one .pte under examples/qualcomm/oss_scripts/llama for checking llama BC changes.

Hi @cccclai
I want to confirm is including the .pte file directly in the codebase what you were expecting? The story.pte file is over 100 MB, so I wanted to double-check if that aligns with your expectation.
Thanks!!

@cccclai
Copy link
Contributor

cccclai commented Jul 21, 2025

Also seems like lots of changes with recent PR...can we prioritize checking a stories.pte file as part of CI so we can know if there is any BC breaking changes

We currently have compile_only and pre_gen_pte flags in TestExampleLLMScript to help check for BC such as compiling with the n-1 version and running on the current (n) version. It would be great to align on where the artifact(pte path) should live.

I think we can put them in a unified folder under qualcomm/backends for the qnn schema related BC checking, and have one .pte under examples/qualcomm/oss_scripts/llama for checking llama BC changes.

Hi @cccclai I want to confirm is including the .pte file directly in the codebase what you were expecting? The story.pte file is over 100 MB, so I wanted to double-check if that aligns with your expectation. Thanks!!

Hmm good point, it's in a critical path currently, and I'd say let's have it here first with a read me to tell others how to update it in the future. Maybe we can upload it to huggingface, I'll chat with my team

@DannyYuyang-quic
Copy link
Collaborator Author

DannyYuyang-quic commented Jul 23, 2025 •
edited
Loading

Hi @cccclai,

I’ve created a draft PR to test the stories15M.pt BC (#12748 ), and it seems to be working properly.
log:
image

While testing, I noticed there’s a smaller story llama with only 260K parameters. However, its voc_size is 512 instead of the usual 32,000.

If you're okay with using stories260K.pt as a benchmark for BC validation on LLaMA, I can try to update the PR to use it instead.

Thanks!

@cccclai
Copy link
Contributor

cccclai commented Jul 23, 2025

Yes stories15M sounds good! Thank you

@DannyYuyang-quic
Qualcomm AI Engine Direct - Phase out QCIR flow since it is no longer…
f2aa739 
… used

Summary:
 - Remove all QCIR-related files and associated custom protocol
@DannyYuyang-quic DannyYuyang-quic force-pushed the dev1/danny/deprecate_qcir_and_CustomProtocol branch from 42000f1 to f2aa739 Compare July 28, 2025 02:13
@facebook-github-bot
Copy link
Contributor

@cccclai has imported this pull request. If you are a Meta employee, you can view this in D79106302.

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

@larryliu0820 larryliu0820 Awaiting requested review from larryliu0820 larryliu0820 is a code owner

@kirklandsign kirklandsign Awaiting requested review from kirklandsign kirklandsign is a code owner

@cccclai cccclai Awaiting requested review from cccclai cccclai is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: qualcomm Changes to the Qualcomm backend delegate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DannyYuyang-quic @cccclai @haowhsu-quic @facebook-github-bot