Skip to content

Add mixed integer precision test for op_mul #11206

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 122 commits into from
Jun 9, 2025
Merged

Conversation

swolchok
Copy link
Contributor

This tests a possibly-surprising result: int8(100) * int8(100) with output type of long is 16 in ATen, even though the output type can hold 10000.

swolchok added 30 commits March 18, 2025 17:32
[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]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
swolchok added 6 commits May 29, 2025 12:23
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@manuelcandales
Copy link
Contributor

wait what? I dont get why is it 16?

because 100 * 100 mod 256 = 16

this is how ATen works, because of functionalization
mul.out(x, y, out=out) must be equivalent to z = mul(x, y); out = z.to(out.dtype)
if x & y are int8, then z is also int8, and so multiplication turns into multiplication modulo 256
this is the downside of functionalization, it is sort of a compromise

swolchok added 6 commits June 9, 2025 12:04
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
Base automatically changed from gh/swolchok/395/head to main June 9, 2025 20:23
@swolchok swolchok merged commit 3a4ec6e into main Jun 9, 2025
97 of 186 checks passed
@swolchok swolchok deleted the gh/swolchok/440/head branch June 9, 2025 20:27
JacobSzwejbka added a commit that referenced this pull request Jun 10, 2025
swolchok added a commit that referenced this pull request Jun 24, 2025
…portedTensorDtypes::BOOL (#9584)", new op_mul test (#11206)

These were reverted because they were part of a stack with interenal test failures.

Original #9585 summary:
We were requiring ourselves to compute at double-precision, but ATen
actually converts non-floating-point types to `float` by default, not
`double`. Use the ATen tolerances everywhere.

Original #9584 summary: none

Original #11206 summary:
This tests a possibly-surprising result: int8(100) * int8(100) with
output type of long is 16 in ATen, even though the output type can hold 10000.

Differential Revision: [D76754823](https://our.internmc.facebook.com/intern/diff/D76754823/)

[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jun 24, 2025
…portedTensorDtypes::BOOL (#9584)", new op_mul test (#11206)

These were reverted because they were part of a stack with interenal test failures.

Original #9585 summary:
We were requiring ourselves to compute at double-precision, but ATen
actually converts non-floating-point types to `float` by default, not
`double`. Use the ATen tolerances everywhere.

Original #9584 summary: none

Original #11206 summary:
This tests a possibly-surprising result: int8(100) * int8(100) with
output type of long is 16 in ATen, even though the output type can hold 10000.

Differential Revision: [D76754823](https://our.internmc.facebook.com/intern/diff/D76754823/)

[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jun 24, 2025
…portedTensorDtypes::BOOL (#9584)", new op_mul test (#11206)

These were reverted because they were part of a stack with interenal test failures.

Original #9585 summary:
We were requiring ourselves to compute at double-precision, but ATen
actually converts non-floating-point types to `float` by default, not
`double`. Use the ATen tolerances everywhere.

Original #9584 summary: none

Original #11206 summary:
This tests a possibly-surprising result: int8(100) * int8(100) with
output type of long is 16 in ATen, even though the output type can hold 10000.

Differential Revision: [D76754823](https://our.internmc.facebook.com/intern/diff/D76754823/)

[ghstack-poisoned]
swolchok added a commit that referenced this pull request Jun 24, 2025
…portedTensorDtypes::BOOL (#9584)", new op_mul test (#11206)

These were reverted because they were part of a stack with interenal test failures.

Original #9585 summary:
We were requiring ourselves to compute at double-precision, but ATen
actually converts non-floating-point types to `float` by default, not
`double`. Use the ATen tolerances everywhere.

Original #9584 summary: none

Original #11206 summary:
This tests a possibly-surprising result: int8(100) * int8(100) with
output type of long is 16 in ATen, even though the output type can hold 10000.

Differential Revision: [D76754823](https://our.internmc.facebook.com/intern/diff/D76754823/)

[ghstack-poisoned]
kedarnath03 pushed a commit to kedarnath03/executorch that referenced this pull request Jun 25, 2025
This tests a possibly-surprising result: int8(100) * int8(100) with output type of long is 16 in ATen, even though the output type can hold 10000.


ghstack-source-id: c946ff3
ghstack-comment-id: 2917867829
Pull-Request-resolved: pytorch/executorch#11206
facebook-github-bot pushed a commit that referenced this pull request Jun 25, 2025
…portedTensorDtypes::BOOL (#9584)", new op_mul test (#11206)

Differential Revision: D76754823

Pull Request resolved: #11942
swolchok added a commit that referenced this pull request Jun 26, 2025
…portedTensorDtypes::BOOL (#9584)", new op_mul test (#11206) (#11976)

This PR was created by the merge bot to help merge the original PR into
the main branch.
ghstack PR number: #11942 by
@swolchok
^ Please use this as the source of truth for the PR details, comments,
and reviews
ghstack PR base:
https://github.com/pytorch/executorch/tree/gh/swolchok/473/base
ghstack PR head:
https://github.com/pytorch/executorch/tree/gh/swolchok/473/head
Merge bot PR base: https://github.com/pytorch/executorch/tree/main
Merge bot PR head:
https://github.com/pytorch/executorch/tree/gh/swolchok/473/orig
@diff-train-skip-merge

Co-authored-by: Scott Wolchok <[email protected]>
hinriksnaer pushed a commit to hinriksnaer/executorch that referenced this pull request Jun 26, 2025
…Add SupportedTensorDtypes::BOOL (pytorch#9584)", new op_mul test (pytorch#11206) (pytorch#11976)

This PR was created by the merge bot to help merge the original PR into
the main branch.
ghstack PR number: pytorch#11942 by
@swolchok
^ Please use this as the source of truth for the PR details, comments,
and reviews
ghstack PR base:
https://github.com/pytorch/executorch/tree/gh/swolchok/473/base
ghstack PR head:
https://github.com/pytorch/executorch/tree/gh/swolchok/473/head
Merge bot PR base: https://github.com/pytorch/executorch/tree/main
Merge bot PR head:
https://github.com/pytorch/executorch/tree/gh/swolchok/473/orig
@diff-train-skip-merge

Co-authored-by: Scott Wolchok <[email protected]>
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.

4 participants