Skip to content

[FX][Quant] Enable FX quant for patterns like x.view(x.size(...), ...) #90001

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
Closed
Xia-Weiwen wants to merge 1 commit into from
Closed

[FX][Quant] Enable FX quant for patterns like x.view(x.size(...), ...) #90001

Xia-Weiwen wants to merge 1 commit into from

Conversation

@Xia-Weiwen
Copy link
Collaborator

@Xia-Weiwen Xia-Weiwen commented Dec 1, 2022 •
edited
Loading

Summary
This work continues with #83784 by @vkuzo and includes all the changes in that PR.
Quote from #83784:

Issue #83658 reports that ops followed by a certain pattern of view and size ops were not quantized correctly by FX graph mode quantization.
Before this PR, the "size" op was in the "op shares qparams with input" category, and the code assumed that the input of this op has the same dtype as its output. This led to incorrectly propagating the int dtype as the output of whichever op was preceding the view op, which in turn made that op blocklisted from quantization.

The fix is to create a new category of ops which work on different dtypes of tensors but are not observed. This PR does so for size, and also for shape since it works the same way.

Note: This PR needs #91297 to be landed first otherwise there is a UT failure.

Test plan

python test/test_quantization.py -k test_linear_size_view
python test/test_quantization.py -k test_linear_shape_view

cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10

@pytorch-bot
Copy link

pytorch-bot bot commented Dec 1, 2022 •
edited
Loading

🔗 Helpful Links

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

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

✅ No Failures

As of commit 5adc1b8:
💚 Looks good so far! There are no failures yet. 💚

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

@pytorch-bot pytorch-bot bot added the release notes: quantization release notes category label Dec 1, 2022
@Xia-Weiwen Xia-Weiwen requested review from jgong5 and vkuzo December 1, 2022 10:11
@Xia-Weiwen Xia-Weiwen mentioned this pull request Dec 1, 2022
Closed
jgong5
jgong5 approved these changes Dec 1, 2022
View reviewed changes
Copy link
Collaborator

@jgong5 jgong5 left a comment

Choose a reason for hiding this comment

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

LGTM. A separate question on the backend configuration. It seems awkward to me that we have to add such a common configuration to all the backends. Maybe we can have a group of common configurations that can be applied directly to all the backends?

@Xia-Weiwen
Copy link
Collaborator Author

Xia-Weiwen commented Dec 2, 2022

LGTM. A separate question on the backend configuration. It seems awkward to me that we have to add such a common configuration to all the backends. Maybe we can have a group of common configurations that can be applied directly to all the backends?

Yes. Maybe we do it in another PR.

@Xia-Weiwen Xia-Weiwen marked this pull request as ready for review December 2, 2022 00:42
@Xia-Weiwen Xia-Weiwen changed the title [FX][Quant] Enable FX quant for pattern like x.view(x.size(...), ...) [FX][Quant] Enable FX quant for patterns like x.view(x.size(...), ...) Dec 2, 2022
@Xia-Weiwen Xia-Weiwen added the intel This tag is for PR from Intel label Dec 2, 2022
@Xia-Weiwen
Copy link
Collaborator Author

Xia-Weiwen commented Dec 14, 2022

Hi @vkuzo @jerryzh168 @z-a-f Could you help review this? Thanks!

jerryzh168
jerryzh168 reviewed Dec 14, 2022
View reviewed changes
torch/ao/quantization/fx/_lower_to_native_backend.py Outdated
Copy link
Contributor

@jerryzh168 jerryzh168 Dec 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

nit: this can also be:

if not is_get_tensor_info_node(n):
    continue

to reduce indentation

Copy link
Collaborator Author

@Xia-Weiwen Xia-Weiwen Dec 15, 2022

Choose a reason for hiding this comment

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

OK. It's fixed

jerryzh168
jerryzh168 approved these changes Dec 15, 2022
View reviewed changes
Copy link
Contributor

@jerryzh168 jerryzh168 left a comment

Choose a reason for hiding this comment

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

LGTM, maybe ask @vkuzo to take a look as well since he added the original PR

@Xia-Weiwen Xia-Weiwen force-pushed the fx_quant_size_op branch 2 times, most recently from edb212b to 9d6abd7 Compare December 21, 2022 07:37
@jerryzh168
Copy link
Contributor

jerryzh168 commented Jan 23, 2023

Hi @Xia-Weiwen any plans to land this?

@Xia-Weiwen
Copy link
Collaborator Author

Xia-Weiwen commented Jan 26, 2023

Hi @Xia-Weiwen any plans to land this?

Hi @jerryzh168. This PR was waiting for #91297 to be landed first. That's why it is still not landed. I was trying to land #91297 but some unrelated CI checks failed. I will land these two PRs today if no further CI issues.

@Xia-Weiwen
Copy link
Collaborator Author

Xia-Weiwen commented Jan 26, 2023

Hi @Xia-Weiwen any plans to land this?

Hi @jerryzh168. This PR was waiting for #91297 to be landed first. That's why it is still not landed. I was trying to land #91297 but some unrelated CI checks failed. I will land these two PRs today if no further CI issues.

The latest master branch is causing a lot of CI failures right now. Probably due to this commit 46f16b9. I will merge this tomorrow if CI checks pass.

@Xia-Weiwen
Copy link
Collaborator Author

Xia-Weiwen commented Jan 27, 2023

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jan 27, 2023
@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Jan 27, 2023

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@Xia-Weiwen Xia-Weiwen deleted the fx_quant_size_op branch November 13, 2024 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jerryzh168 jerryzh168 jerryzh168 approved these changes

@jgong5 jgong5 jgong5 approved these changes

@vkuzo vkuzo Awaiting requested review from vkuzo

@z-a-f z-a-f Awaiting requested review from z-a-f

Assignees

No one assigned

Labels

ciflow/trunk Trigger trunk jobs on your pull request intel This tag is for PR from Intel Merged open source release notes: quantization release notes category

Projects

PyTorch Intel
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Xia-Weiwen @jerryzh168 @pytorchmergebot @jgong5 @pytorchbot