Skip to content

Qualcomm AI Engine Direct - fix LPBQ implementation #12663

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.

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

Merged
merged 1 commit into from
Jul 21, 2025

Conversation

haowhsu-quic
Copy link
Collaborator

Summary

  • fix LPBQ and make test case more general

Test plan

python backends/qualcomm/tests/test_qnn_delegate.py TestQNNQuantizedOperator.test_qnn_backend_conv2d_block -b build-android -s $DEVICE -m SM8750

- fix LPBQ and make test case more general
@haowhsu-quic haowhsu-quic requested a review from cccclai as a code owner July 19, 2025 14:09
Copy link

pytorch-bot bot commented Jul 19, 2025

🔗 Helpful Links

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

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

❌ 2 New Failures

As of commit e8ad73c with merge base 4d7f9ca (image):

NEW FAILURES - The following jobs have failed:

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 19, 2025
Copy link

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@haowhsu-quic
Copy link
Collaborator Author

haowhsu-quic commented Jul 19, 2025

Hi @cccclai, I made a mistake when implementing LPBQ. I've tested current version against Llama3.2 1B and can get better results now. Will update the related change on llama.py in another PR.

@cccclai
Copy link
Contributor

cccclai commented Jul 20, 2025

Hi @cccclai, I made a mistake when implementing LPBQ. I've tested current version against Llama3.2 1B and can get better results now. Will update the related change on llama.py in another PR.

Thanks! Mind sharing a bit more details about the mistake? Trying to follow the code but not super clear

@haowhsu-quic
Copy link
Collaborator Author

haowhsu-quic commented Jul 20, 2025

Yes, currently HTP uses 4bits to store quantized scales. I didn't clip the numeric to the right range (before: 0-255 / after: 1-16). I also double check with AIMET's implementation and the mse between per-channel / per-block.

@cccclai
Copy link
Contributor

cccclai commented Jul 20, 2025

Yes, currently HTP uses 4bits to store quantized scales. I didn't clip the numeric to the right range (before: 0-255 / after: 1-16). I also double check with AIMET's implementation and the mse between per-channel / per-block.

Ah that makes sense..

Copy link
Contributor

@cccclai cccclai left a comment

Choose a reason for hiding this comment

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

Thanks for the fix

@facebook-github-bot
Copy link
Contributor

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

@cccclai cccclai merged commit 0fbd6d4 into pytorch:main Jul 21, 2025
100 of 103 checks passed
Conarnar pushed a commit to Conarnar/executorch that referenced this pull request Jul 25, 2025
### Summary
- fix LPBQ and make test case more general

### Test plan
```python
python backends/qualcomm/tests/test_qnn_delegate.py TestQNNQuantizedOperator.test_qnn_backend_conv2d_block -b build-android -s $DEVICE -m SM8750
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants