Improve handling of mixed-type tensor operations #22273

nairbv · 2019-06-26T19:55:00Z

Improve handling of mixed-type tensor operations.

This PR affects the arithmetic (add, sub, mul, and div) operators implemented via TensorIterator (so dense but not sparse tensor ops).

For these operators, we will now promote to reasonable types where possible, following the rules defined in #9515, and error in cases where the cast would require floating point -> integral or non-boolean to boolean downcasts.

The details of the promotion rules are described here:
https://github.com/nairbv/pytorch/blob/promote_types_strict/docs/source/tensor_attributes.rst

Some specific backwards incompatible examples:

now int_tensor * float will result in a float tensor, whereas previously the floating point operand was first cast to an int. Previously torch.tensor(10) * 1.9 => tensor(10) because the 1.9 was downcast to 1. Now the result will be the more intuitive tensor(19)
Now int_tensor *= float will error, since the floating point result of this operation can't be cast into the in-place integral type result.

See more examples/detail in the original issue (#9515), in the above linked tensor_attributes.rst doc, or in the test_type_promotion.py tests added in this PR:
https://github.com/nairbv/pytorch/blob/promote_types_strict/test/test_type_promotion.py

colesbury

@gchanan's suggestion, which I agree with, is to make int_tensor *= float an error and then see if it triggers any errors in the PyTorch examples and whatever Will is using to test the variable/tensor merge. If there are cases that trigger the error, we should support the "legacy" behavior with a deprecation warning.

aten/src/ATen/native/TensorIterator.cpp

tools/autograd/derivatives.yaml

torch/csrc/jit/passes/shape_analysis.cpp

nairbv · 2019-07-08T20:26:39Z

@pytorchbot rebase this please

pytorchbot · 2019-07-08T20:26:46Z

Sorry, I can't merge this because there are conflicts. To merge this yourself, run the commands below:

git fetch origin master
git fetch [email protected]:nairbv/pytorch.git promote_types_strict
git checkout FETCH_HEAD
git merge origin/master
git push [email protected]:nairbv/pytorch.git HEAD:promote_types_strict

(To learn more about this bot, see Bot commands.)

…ghan/repos/pytorch into promote_types_strict

…osals_op

nairbv · 2019-07-09T20:49:57Z

@wanchaol / @eellison any thoughts on the shape analysis changes here?

Also @ailzhang, this PR would cause a break in XLA, is it anything that would be difficult to fix?

ailzhang · 2019-07-09T20:59:55Z

@nairbv Thanks for letting us know! To me it sounds like we are changing returned types by type promotion and it shouldn't be hard for XLA to follow our behaviors. cc @dlibenzi @asuhan
@nairbv To make it easier to understand, would you mind adding a summary what exactly this PR do?
e.g. under what condition, what type promotion is triggered.
I think it'll be super helpful for XLA to follow the changes, and understand our overall plan on this. Thanks!

asuhan · 2019-07-09T21:14:21Z

@ailzhang @nairbv I don't see major issues on XLA end with doing less implicit conversions. If anything, it was more effort for us to add implicit conversion logic to be as forgiving as other backends (CPU or CUDA), XLA doesn't do implicit conversions on its own.

As far as I can tell, the errors in our integration are in the test framework itself, when we're comparing whether output and expected are close. We can adjust that quite easily.

nairbv · 2019-07-09T21:28:34Z

would you mind adding a summary what exactly this PR do? e.g. under what condition, what type promotion is triggered.

updated the description to contain more info from the linked PR and issue, and linked directly to the issue.

This is still a work-in-progress, ironing out details of the change and doing more testing, but we'll let know again before merging.

nairbv · 2019-09-03T14:18:46Z

@pytorchbot rebase this please

facebook-github-bot

@nairbv has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

nairbv · 2019-09-03T18:12:06Z

@pytorchbot retest this please

nairbv · 2019-09-03T18:42:39Z

@pytorchbot rebase this please

facebook-github-bot

@nairbv has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

… promote_types_strict

facebook-github-bot

@nairbv has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

nairbv · 2019-09-05T13:44:41Z

@pytorchbot rebase this please

pytorchbot · 2019-09-05T13:44:45Z

There's nothing to do! This branch is already up to date with master (a3d0abf).

(To learn more about this bot, see Bot commands.)

facebook-github-bot

@nairbv has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: Improve handling of mixed-type tensor operations. This PR affects the arithmetic (add, sub, mul, and div) operators implemented via TensorIterator (so dense but not sparse tensor ops). For these operators, we will now promote to reasonable types where possible, following the rules defined in #9515, and error in cases where the cast would require floating point -> integral or non-boolean to boolean downcasts. The details of the promotion rules are described here: https://github.com/nairbv/pytorch/blob/promote_types_strict/docs/source/tensor_attributes.rst Some specific backwards incompatible examples: * now `int_tensor * float` will result in a float tensor, whereas previously the floating point operand was first cast to an int. Previously `torch.tensor(10) * 1.9` => `tensor(10)` because the 1.9 was downcast to `1`. Now the result will be the more intuitive `tensor(19)` * Now `int_tensor *= float` will error, since the floating point result of this operation can't be cast into the in-place integral type result. See more examples/detail in the original issue (#9515), in the above linked tensor_attributes.rst doc, or in the test_type_promotion.py tests added in this PR: https://github.com/nairbv/pytorch/blob/promote_types_strict/test/test_type_promotion.py Pull Request resolved: #22273 Reviewed By: gchanan Differential Revision: D16582230 Pulled By: nairbv fbshipit-source-id: 4029cca891908cdbf4253e4513c617bba7306cb3

Summary: Improve handling of mixed-type tensor operations. This PR affects the arithmetic (add, sub, mul, and div) operators implemented via TensorIterator (so dense but not sparse tensor ops). For these operators, we will now promote to reasonable types where possible, following the rules defined in pytorch/pytorch#9515, and error in cases where the cast would require floating point -> integral or non-boolean to boolean downcasts. The details of the promotion rules are described here: https://github.com/nairbv/pytorch/blob/promote_types_strict/docs/source/tensor_attributes.rst Some specific backwards incompatible examples: * now `int_tensor * float` will result in a float tensor, whereas previously the floating point operand was first cast to an int. Previously `torch.tensor(10) * 1.9` => `tensor(10)` because the 1.9 was downcast to `1`. Now the result will be the more intuitive `tensor(19)` * Now `int_tensor *= float` will error, since the floating point result of this operation can't be cast into the in-place integral type result. See more examples/detail in the original issue (pytorch/pytorch#9515), in the above linked tensor_attributes.rst doc, or in the test_type_promotion.py tests added in this PR: https://github.com/nairbv/pytorch/blob/promote_types_strict/test/test_type_promotion.py Pull Request resolved: pytorch/pytorch#22273 Reviewed By: gchanan Differential Revision: D16582230 Pulled By: nairbv fbshipit-source-id: 4029cca891908cdbf4253e4513c617bba7306cb3

Summary: test_wrapped_number was calling torch.set_default_tensor_type('torch.FloatTensor'), which was setting the default tensor types for all following tests until a class boundary (with unittest) or until end of file (with pytest). Tests that don't expect the default tensor type to be set this way were then failing if run afterwards. This fixes the issue by copying the default_tensor_type decorator from test_nn and using that instead with test_wrapped_number. The decorator correctly resets the default tensor type after the test has run. This fixes the many errors encountered when running pytest test_jit.py. Note: test_wrapped_number was introduced in #22273. Pull Request resolved: #26523 Differential Revision: D17495283 Pulled By: mruberry fbshipit-source-id: ab518c78b7706af7cb1c2d1c17823d311178996d

nairbv added 3 commits June 19, 2019 11:15

Enable promotion tensor types in tensoriterator operations.

dfbd623

add test

d710571

type promotion with strict behavior

1d5ecdd

pytorchbot added module: cuda Related to torch.cuda, and CUDA support in general module: internals Related to internal abstractions in c10 and ATen module: operators module: tests Issues related to tests (not the torch.testing module) labels Jun 26, 2019

nairbv added the module: bc-breaking Related to a BC-breaking change label Jun 26, 2019

nairbv changed the title ~~Promote tensor types~~ Promote tensor types without unsafe operations Jun 26, 2019

nairbv mentioned this pull request Jun 26, 2019

Enable promotion of tensor types in TensorIterator operations. #21969

Closed

colesbury reviewed Jun 26, 2019

View reviewed changes

aten/src/ATen/native/TensorIterator.cpp Outdated Show resolved Hide resolved

aten/src/ATen/native/TensorIterator.cpp Outdated Show resolved Hide resolved

aten/src/ATen/native/TensorIterator.cpp Outdated Show resolved Hide resolved

handle promote-types cases described by @colesbury and fix some tests

50c2d2f

pytorchbot added oncall: jit Add this issue/PR to JIT oncall triage queue module: autograd Related to torch.autograd, and the autograd engine in general module: cpu CPU specific problem (e.g., perf, algorithm) labels Jul 3, 2019

nairbv commented Jul 3, 2019

View reviewed changes

tools/autograd/derivatives.yaml Outdated Show resolved Hide resolved

nairbv commented Jul 3, 2019

View reviewed changes

torch/csrc/jit/passes/shape_analysis.cpp Outdated Show resolved Hide resolved

nairbv commented Jul 3, 2019

View reviewed changes

torch/csrc/jit/passes/shape_analysis.cpp Outdated Show resolved Hide resolved

fix test to match new error message

9fd3842

nairbv added 5 commits July 8, 2019 13:35

Merge remote-tracking branch 'origin/master' into HEAD

532d08a

fix flake/tidy issues

5ca2895

fix python2/3 compatibility issue

9ae446e

Merge branch 'promote_types_strict' of ssh://devvm1185.frc1/home/bvau…

4d6936f

…ghan/repos/pytorch into promote_types_strict

fix failing TorchIntegration.test_collect_and_distribute_fpn_rpn_prop…

af9f4dd

…osals_op

pytorchbot and others added 2 commits September 3, 2019 14:18

Merge remote-tracking branch 'origin/master' into HEAD

5d44ce7

only allow boolean alpha for boolean add/sub result

b53220c

facebook-github-bot reviewed Sep 3, 2019

View reviewed changes

Merge remote-tracking branch 'origin/master' into HEAD

ee708f1

facebook-github-bot reviewed Sep 3, 2019

View reviewed changes

ifedan mentioned this pull request Sep 4, 2019

[fix] quantized_tensor tests #25429

Closed

nairbv added 2 commits September 4, 2019 11:23

remove a test that no longer throws due to change in engine.cpp

d0f4da3

Merge branch 'promote_types_strict' of github.com:nairbv/pytorch into…

b6c17a3

… promote_types_strict

nairbv requested review from ebetica, goldsborough and yf225 as code owners September 4, 2019 15:25

pytorchbot added the module: cpp Related to C++ API label Sep 4, 2019

facebook-github-bot reviewed Sep 4, 2019

View reviewed changes

ailzhang mentioned this pull request Sep 4, 2019

promote torch.prod result to int64 for int8 inputs pytorch/xla#863

Closed

Merge branch 'master' into promote_types_strict

e176c5c

facebook-github-bot reviewed Sep 5, 2019

View reviewed changes

facebook-github-bot closed this in facebookresearch/pytext@aa1f9de Sep 6, 2019

ailzhang mentioned this pull request Sep 6, 2019

fix pred promotion for arithmetic ops pytorch/xla#1009

Merged

ifedan mentioned this pull request Sep 10, 2019

compute common dtype based on inputs only #25593

Closed

peterbell10 mentioned this pull request Sep 17, 2019

Rewrite argmax and argmin as TensorIterator reductions #26181

Closed

mruberry mentioned this pull request Sep 20, 2019

Fixes test_wrapped_number #26523

Closed

Improve handling of mixed-type tensor operations #22273

Improve handling of mixed-type tensor operations #22273

Uh oh!

Conversation

nairbv commented Jun 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

colesbury left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nairbv commented Jul 8, 2019

Uh oh!

pytorchbot commented Jul 8, 2019

Uh oh!

nairbv commented Jul 9, 2019

Uh oh!

ailzhang commented Jul 9, 2019

Uh oh!

asuhan commented Jul 9, 2019

Uh oh!

nairbv commented Jul 9, 2019

Uh oh!

nairbv commented Sep 3, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

nairbv commented Sep 3, 2019

Uh oh!

nairbv commented Sep 3, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

nairbv commented Sep 5, 2019

Uh oh!

pytorchbot commented Sep 5, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

nairbv commented Jun 26, 2019 •

edited

Loading