Skip to content

Quantize compatible node + activation patterns as one block #7555

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

Conversation

Tessil
Copy link
Collaborator

@Tessil Tessil commented Jan 8, 2025

Annotate conv1d/conv2d/linear followed by relu/relu6 patterns as one block and fuse the activation into its parent.

The activation will then be implicitly done in the tosa.rescale node that will have a -128 zero-point.

@Tessil Tessil added the partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm label Jan 8, 2025
Copy link

pytorch-bot bot commented Jan 8, 2025

🔗 Helpful Links

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

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

✅ No Failures

As of commit b60972b with merge base 08770b7 (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 Jan 8, 2025
Annotate conv1d/conv2d/linear followed by relu/relu6 patterns as one block and fuse the activation into its parent. The activation will then be implicitly done in the tosa.rescale node that will have a -128 zero-point.

Change-Id: I5bf1e2c91be21ab842012fbc20d159af7fe2222d
@freddan80 freddan80 merged commit a059981 into pytorch:main Jan 10, 2025
165 of 173 checks passed
@swolchok
Copy link
Contributor

This PR caused a type-checking failure:

executorch/backends/arm/_passes/fuse_quantized_activation_pass.py:27:48 Uninitialized local [61]: Local variable `zp` is undefined, or not always defined.
executorch/backends/arm/_passes/fuse_quantized_activation_pass.py:27:54 Uninitialized local [61]: Local variable `qmin` is undefined, or not always defined.

@freddan80
Copy link
Collaborator

@Tessil will you have a look?

@Tessil
Copy link
Collaborator Author

Tessil commented Jan 14, 2025

@swolchok Thanks for the info. What would be the best way to fix it as the PR has already gone through? Raise a new one with a fix?

Note that the zp and qmin will theoretically never be evaluated if they are uninitialized with the short-circuit and as they are initialized in any case when is_quantized is true and evaluated only if is_quantized is true. Though still best to fix the type-checking issue.

@swolchok
Copy link
Contributor

Raise a new one with a fix?

yep!

@Tessil
Copy link
Collaborator Author

Tessil commented Jan 15, 2025

@swolchok Fix: #7671
Thanks

YIWENX14 pushed a commit that referenced this pull request Jan 28, 2025
Annotate conv1d/conv2d/linear followed by relu/relu6 patterns as one block and fuse the activation into its parent. The activation will then be implicitly done in the tosa.rescale node that will have a -128 zero-point.

Change-Id: I5bf1e2c91be21ab842012fbc20d159af7fe2222d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm topic: not user facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants