Skip to content

[Backend Tester] Add quantized test flows for XNNPACK and Core ML #12733

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 30 commits into from
Jul 23, 2025

Conversation

GregoryComer
Copy link
Member

@GregoryComer GregoryComer commented Jul 22, 2025

Add quantized test flows core static int8 quantization for XNNPACK and Core ML backends. I also ended up doing some light refactoring on the test signature to pass the TestFlow class into the individual tests. This is done to allow for passing quantization parameters into the inner test.

[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
Copy link

pytorch-bot bot commented Jul 22, 2025

🔗 Helpful Links

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

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

⏳ No Failures, 77 Pending

As of commit 27cd171 with merge base 29c52b5 (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 Jul 22, 2025
[ghstack-poisoned]

import coremltools as ct
Copy link
Contributor

Choose a reason for hiding this comment

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

come on :p

Suggested change
import coremltools as ct
import coremltools

def _get_static_int8_qconfig():
return ct.optimize.torch.quantization.LinearQuantizerConfig(
global_config=ct.optimize.torch.quantization.ModuleLinearQuantizerConfig(
quantization_scheme="symmetric",
Copy link
Contributor

@digantdesai digantdesai Jul 23, 2025

Choose a reason for hiding this comment

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

is this the main int8 schema we should be testing for Linear @metascroy

Copy link
Member Author

Choose a reason for hiding this comment

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

FYI This is pulled directly from our docs at https://docs.pytorch.org/executorch/main/backends-coreml.html#bit-quantization-using-the-pt2e-flow. Would be good to sanity check with Scott, though.

logger.info("Skipping XNNPACK flow registration due to import failure.")
return None

from executorch.backends.test.suite.flows.xnnpack import XNNPACK_TEST_FLOW, XNNPACK_STATIC_INT8_TEST_FLOW
Copy link
Contributor

Choose a reason for hiding this comment

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

from ... import *?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to clean up flow registration slightly in the stack, so I'll take this as a follow-up there.

Comment on lines 29 to 33
quantize: bool = field(default=False)
""" Whether to tester should run the quantize stage on the model. """

quantize_stage_factory: Callable[..., Quantize] | None = None
""" A factory function which instantiates a Quantize stage. Can be None to use the tester's default. """
Copy link
Contributor

Choose a reason for hiding this comment

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

why an extra flag?

Copy link
Member Author

Choose a reason for hiding this comment

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

The specific reason is that if quantize_stage_factory isn't provided, it will use the default Quantize stage from the tester. I could maybe just always require the caller to provide quantize_stage_factory.

@@ -14,23 +14,26 @@ class TestResult(IntEnum):

EAGER_FAIL = 2
""" The test failed due to the model failing to run in eager mode. """

QUANTIZE_FAIL = 3
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to distinguish export_for_trainint (old) before quantize and export after quantize?

Copy link
Member Author

Choose a reason for hiding this comment

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

If I'm following correctly, are you asking about catching errors in the quant export vs the quantization itself?

It would be nice to have this, though we'd need to refactor the tester a bit to split it.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah finer grained. We can do this later, hence the stamp :)

Copy link
Contributor

@digantdesai digantdesai left a comment

Choose a reason for hiding this comment

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

you can split quantizer and the using flow into two PRs but OK too.

[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@GregoryComer
Copy link
Member Author

GregoryComer commented Jul 23, 2025

Closes #12494

[ghstack-poisoned]
@GregoryComer GregoryComer changed the base branch from gh/GregoryComer/80/head to main July 23, 2025 21:21
[ghstack-poisoned]
GregoryComer added a commit that referenced this pull request Jul 23, 2025
ghstack-source-id: 3bd0358
ghstack-comment-id: 3105090683
Pull-Request: #12733
@GregoryComer
Copy link
Member Author

Updated to address comments and lints. There are a few minor follow-ups (see specific comment chains), but I'm intending to land and take follows-up after to keep things moving.

[ghstack-poisoned]
@GregoryComer
Copy link
Member Author

Unit test failures are pre-existing on main (pytest coverage error). CI was green before rebasing. Merging.

@GregoryComer GregoryComer merged commit 1af1d11 into main Jul 23, 2025
100 of 104 checks passed
@GregoryComer GregoryComer deleted the gh/GregoryComer/83/head branch July 23, 2025 23:28
Conarnar pushed a commit to Conarnar/executorch that referenced this pull request Jul 25, 2025
…torch#12733)

Add quantized test flows core static int8 quantization for XNNPACK and
Core ML backends. I also ended up doing some light refactoring on the
test signature to pass the TestFlow class into the individual tests.
This is done to allow for passing quantization parameters into the inner
test.
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. release notes: none Do not include this in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants