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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NNC] Some ops have type promotion logic which adds extra casts & does compute in different dtype than eager #49178

Open
1 task
eellison opened this issue Dec 10, 2020 · 1 comment
Labels
module: bootcamp We plan to do a full writeup on the issue, and then get someone to do it for onboarding NNC triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects

Comments

@eellison
Copy link
Contributor

eellison commented Dec 10, 2020

馃悰 Bug

Many NNC type promotion goes through a common path which promotes inputs according to the highest input type, and then after compute, casts the output to whichever output type was recorded when the op was run in eager.

Because we have the output type specified, all NNC ops will give the correct dtype. However, the compute may be done in a different dtype than eager, and there may be extraneous casts added.

In the (worst) case of something like,

x = torch.ones([4,  4], dtype=torch.int16)
torch.addcmul(x, x, x, value=1)

We will cast all three tensor inputs to int32, do the addcmul, then cast the operation back. As opposed to eager, which will just cast value=1 to int16.

The advantage of the current approach is that we are guaranteed to have the same output dtype. I think ops could gradually be migrated over to have the correct casting behavior.

Most ops do promote to the highest dtype, so this isn't that big of an issue, but it does come up.

TODO:

  • torch.addcmul (add your name here to claim)

To Reproduce

  1. Comment out the output type casting of the various comput{number}Operands

  2. Run python test/test_jit_fuser_te..py

@eellison eellison added module: bootcamp We plan to do a full writeup on the issue, and then get someone to do it for onboarding NNC labels Dec 10, 2020
@eellison
Copy link
Contributor Author

Adding bootcamp here because each individual op is pretty easy to migrate over to the right casting logic. Starting with addcmul but we can add others.

@H-Huang H-Huang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Dec 10, 2020
@eellison eellison changed the title [NNC] NNC type promotion logic adds extra casts & does compute in different dtype than eager [NNC] Some ops have type promotion logic which adds extra casts & does compute in different dtype than eager Dec 10, 2020
@ZolotukhinM ZolotukhinM added this to High priority in NNC Feb 22, 2021
@ZolotukhinM ZolotukhinM moved this from High priority to Needs triage in NNC Feb 22, 2021
@ZolotukhinM ZolotukhinM moved this from Needs triage to High priority in NNC Feb 22, 2021
@ZolotukhinM ZolotukhinM moved this from High priority to Low priority in NNC Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: bootcamp We plan to do a full writeup on the issue, and then get someone to do it for onboarding NNC triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
NNC
Low priority
Development

No branches or pull requests

2 participants