Skip to content
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

Check and fix dequantize_affine is idempotent #309

Merged
merged 1 commit into from
Jun 3, 2024
Merged

Conversation

cpuhrsch
Copy link
Contributor

@cpuhrsch cpuhrsch commented Jun 3, 2024

Resolves #289

Follow up includes adding this check for quantize_affine as well.

Follow up includes making tests run on 2.3+ instead of 2.4+ by porting reference into torchao.

Follow up includes deduplicating the test code and creating reference IR to catch compile regressions.

Copy link

pytorch-bot bot commented Jun 3, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit a064a83 with merge base 88daa1a (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 Jun 3, 2024
@cpuhrsch cpuhrsch requested a review from msaroufim June 3, 2024 20:31
@cpuhrsch cpuhrsch marked this pull request as ready for review June 3, 2024 20:31
@cpuhrsch
Copy link
Contributor Author

cpuhrsch commented Jun 3, 2024

This may result in a performance regression due to the extra copy (even though I'd expect the compiler to handle this). However, the existing implementation can also yield silent correctness issues in case the input is being fed into multiple modules that use this dtype or when using skip connections, etc.

@cpuhrsch cpuhrsch merged commit d75f450 into main Jun 3, 2024
13 checks passed
dbyoung18 pushed a commit to dbyoung18/ao that referenced this pull request Jul 31, 2024
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.

dequantize_affine modified the input in-place
3 participants