Add tensor.view(dtype) #47951

zasdfgbnm · 2020-11-14T00:13:08Z

Fixes #42571

Note that this functionality is a subset of numpy.ndarray.view:

this only supports viewing a tensor as a dtype with the same number of bytes
this does not support viewing a tensor as a subclass of torch.Tensor

zasdfgbnm · 2020-11-14T00:20:14Z

Oh, sorry, I forget the doc

dr-ci · 2020-11-14T00:20:16Z

💊 CI failures summary and remediations

As of commit a3295fc (more details on the Dr. CI page):

2/2 failures possibly* introduced in this PR
- 1/2 non-CircleCI failure(s)

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

pytorch_windows_vs2019_py36_cuda10.1_test2 (1/1)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

AssertionError: "Simulate error" does not match "grad can be implicitly created only for scalar outputs"


Traceback (most recent call last):
  File "C:\Users\circleci\project\build\win_tmp\build\torch\testing\_internal\common_device_type.py", line 279, in instantiated_test
    result = test_fn(self, *args)
  File "C:\Users\circleci\project\build\win_tmp\build\torch\testing\_internal\common_device_type.py", line 676, in only_fn
    return fn(slf, device, *args, **kwargs)
  File "test_autograd.py", line 6643, in test_reentrant_parent_error_on_cpu
    self._test_reentrant_parent_error_on_cpu(device)
  File "test_autograd.py", line 6629, in _test_reentrant_parent_error_on_cpu
    torch.autograd.backward([t5.sum(), t7.sum()])
AssertionError: "Simulate error" does not match "grad can be implicitly created only for scalar outputs"

----------------------------------------------------------------------
Ran 2847 tests in 1673.906s

FAILED (failures=1, skipped=23, expected failures=1)

Generating XML reports...
Generated XML report: test-reports\python-unittest\TEST-TestAutograd-20210108012914.xml
Generated XML report: test-reports\python-unittest\TEST-TestAutogradComplex-20210108012914.xml
Generated XML report: test-reports\python-unittest\TEST-TestAutogradDeviceTypeCPU-20210108012914.xml

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

This comment has been revised 80 times.

zasdfgbnm · 2020-11-14T04:59:21Z

torch/csrc/jit/ir/ir.cpp

@@ -104,7 +104,7 @@ std::ostream& operator<<(
 static void printAttribute(std::ostream& out, const at::Tensor& tensor) {
  // 1-elem tensors are usually boxed scalars, so print them like it
  if (tensor.numel() == 1) {
-    auto scalar_tensor = tensor.view({}).item();
+    auto scalar_tensor = tensor.view(std::vector<int64_t>{}).item();


Is there a way to stop {} from being resolved to ScalarType?

Do we still need these changes if dtype overload is blocklisted?

Yes, it is blocklisted at runtime for schema matching, but not at compile time as in here.

vadimkantorov · 2020-11-14T20:07:37Z

original related issue: #29013

vadimkantorov · 2020-11-14T20:13:57Z

Despite historic NumPy-originated name "view(...)", maybe a more clear alias name "reinterpret(...)" would be nice as well

zasdfgbnm · 2020-11-15T08:48:06Z

torch/csrc/jit/frontend/schema_matching.cpp

+// Note (@zasdfgbnm):
+// This is a workaround for https://github.com/pytorch/pytorch/issues/47964
+// Currently JIT does not distinguish ScalarType vs int, so there is really
+// no way to distinguish x.view(1) vs x.view(torch.int8). So we have to hardcode
+// the aten::view.dtype here to block this overload. This blocklist should be
+// removed when JIT fully suports ScalarType as its own type.
+bool isBlockListedSchema(const FunctionSchema& schema) {
+  if (schema.name() == "aten::view" && schema.overload_name() == "dtype") {
+    return true;
+  }
+  return false;
+}
+


I looked at the codegen, and some codes in dispatcher and JIT, and I feel that hard coding the operator name here is the best solution to workaround #47964.

It would be nice to allow it work with the unambiguous torch.view(dtype=torch.int8)

Are you suggesting that if schema.name() == "aten::view" then I should check if kwargs is "dtype" and special case it to make this case work? I think this would require changing more places in tryMatchSchema. As a temporary workaround, I prefer the workaround to be simple, and can be easily reverted when the real fix (which is to add ScalarType support to JIT) is landed.

codecov · 2020-11-15T22:54:42Z

Codecov Report

Merging #47951 (cc88ec7) into master (68a6e46) will increase coverage by 0.19%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master   #47951      +/-   ##
==========================================
+ Coverage   80.49%   80.68%   +0.19%     
==========================================
  Files        1900     1900              
  Lines      206305   206318      +13     
==========================================
+ Hits       166056   166470     +414     
+ Misses      40249    39848     -401

ngimel · 2020-11-18T18:45:27Z

cc @albanD for autograd.

mruberry · 2020-11-18T18:48:03Z

cc @bwasti

eellison · 2020-11-18T18:51:10Z

torch/csrc/jit/frontend/schema_matching.cpp

+// Note (@zasdfgbnm):
+// This is a workaround for https://github.com/pytorch/pytorch/issues/47964
+// Currently JIT does not distinguish ScalarType vs int, so there is really
+// no way to distinguish x.view(1) vs x.view(torch.int8). So we have to hardcode


Which overload is x.view(torch.int8) getting matched to without this logic?

Without this logic, both x.view(1) and x.view(torch.int8) get matched to aten::view.dtype. Even x.view(-1) get matched to aten::view.dtype although -1 is not a valid dtype.

Actuacally I think aten::view.dtype gets priority over aten::view because aten::view.dtype can be matched at tryMatchSchema(..., allow_conversions=False), but aten::view is usually matched by tryMatchSchema(..., allow_conversions=True)

facebook-github-bot

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

aten/src/ATen/native/TensorConversions.cpp

albanD · 2020-11-18T18:55:25Z

torch/_tensor_docs.py

+of elements, but may have a different dtype. For a tensor to be viewed, the new
+dtype must have the same number of bytes with its original dtype.
+
+.. warning::


Could we blacklist this overload in torchscript to avoid this kind of confusing error?

albanD · 2020-11-18T18:57:08Z

tools/autograd/derivatives.yaml

@@ -1129,6 +1129,9 @@
 - name: view(Tensor(a) self, int[] size) -> Tensor(a)
  self: grad.reshape(self.sizes())

+- name: view.dtype(Tensor(a) self, ScalarType dtype) -> Tensor(a)
+  output_differentiability: [False]


That will work for now. You can ping me if you want to change that to True in the future :) (we already have similar things for complex dtypes so it shouldn't be too hard to add).

Is the grad of this overload well defined mathematically? For the reinterpreting of two different types?

The int<-->float reinterpret will not work because int tensors do not support gradients. The only reinterpret in question I think is the double<-->complex64. But I don't think this makes sense either. For example, if we change the real part of the complex64 from 0 to 1.1111111111*2^-80, then the exponent bits of the double tensor will be changed, and the limit

lim (f(x+dx) - f(x)) / dx dx->0

don't seem to be converging.

Well, the forward is really defined mathematically either right? haha
But I do agree that we most likely want to keep it non differentiable for now.

zasdfgbnm · 2021-01-05T00:06:29Z

@mruberry I think this PR is ready?

mruberry · 2021-01-06T12:27:09Z

This PR looks good to me, and I think you're right, @zasdfgbnm, but there may be a docs formatting issue. Take a look at the docs artifact here: https://9950312-65600975-gh.circle-artifacts.com/0/docs/tensors.html?highlight=view#torch.Tensor.view. In particular, this line:

view(dtype) -> Tensor

Needs to be rendered like the multiple schema definitions for other operations (see, for example, sum's documentation: https://pytorch.org/docs/master/generated/torch.sum.html?highlight=sum#torch.sum).

Just ping me when that's fixed.

zasdfgbnm · 2021-01-06T18:20:16Z

@mruberry This is fixed

mruberry

LGTM!

ngimel · 2021-01-08T00:00:22Z

aten/src/ATen/native/TensorConversions.cpp

+    "Viewing a tensor as a new dtype with a different number of bytes per element is not supported.");
+  Storage storage = self.storage();
+  auto new_tensor = detail::make_tensor<TensorImpl>(
+      std::move(storage), self.key_set(), type_meta);


@zasdfgbnm do you know what would happen if original tensor required grad, and you are viewing it as integer? Would key_set still have autograd key? The operation is non-differentiable, so it's not particularly important, unless it crashes or produces confusing error message. Can you add a test for what would happen?

It will just return a tensor with requires_grad=False. I have added the test.

ngimel · 2021-01-08T00:08:16Z

torch/csrc/jit/ir/ir.cpp

@@ -104,7 +104,7 @@ std::ostream& operator<<(
 static void printAttribute(std::ostream& out, const at::Tensor& tensor) {
  // 1-elem tensors are usually boxed scalars, so print them like it
  if (tensor.numel() == 1) {
-    auto scalar_tensor = tensor.view({}).item();
+    auto scalar_tensor = tensor.view(std::vector<int64_t>{}).item();


Do we still need these changes if dtype overload is blocklisted?

facebook-github-bot

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

facebook-github-bot · 2021-01-08T15:11:54Z

@mruberry merged this pull request in d00aceb.

Summary: Fixes pytorch#42571 Note that this functionality is a subset of [`numpy.ndarray.view`](https://numpy.org/doc/stable/reference/generated/numpy.ndarray.view.html): - this only supports viewing a tensor as a dtype with the same number of bytes - this does not support viewing a tensor as a subclass of `torch.Tensor` Pull Request resolved: pytorch#47951 Reviewed By: ngimel Differential Revision: D25062301 Pulled By: mruberry fbshipit-source-id: 9fefaaef77f15d5b863ccd12d836932983794475

Add tensor.view(dtype)

213e49c

zasdfgbnm requested review from mruberry and ngimel November 14, 2020 00:13

facebook-github-bot added the cla signed label Nov 14, 2020

pytorchbot added the open source label Nov 14, 2020

, rtol=0, atol=0

c94682e

zasdfgbnm added 3 commits November 13, 2020 16:33

docs

98f6e64

ident

a1b1187

save

9e3233a

zasdfgbnm requested a review from apaszke as a code owner November 14, 2020 04:25

zasdfgbnm commented Nov 14, 2020

View reviewed changes

zasdfgbnm mentioned this pull request Nov 14, 2020

TorchScript is unable to distinguish int and ScalarType #47964

Open

zasdfgbnm added 7 commits November 14, 2020 16:58

Merge branch 'master' of github.com:pytorch/pytorch into view-dtype

09326b7

fix

0dcf5e0

test

de92d21

revert

c510b07

save

89825d9

more warning

4b463a5

overload

c016d43

zasdfgbnm commented Nov 15, 2020

View reviewed changes

zasdfgbnm added 4 commits November 15, 2020 00:50

or

dbcce32

fix

ea5c9f1

clang-format

c452bb5

skip xla

5931a8f

zou3519 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Nov 16, 2020

eellison reviewed Nov 18, 2020

View reviewed changes

facebook-github-bot reviewed Nov 18, 2020

View reviewed changes

albanD reviewed Nov 18, 2020

View reviewed changes

zasdfgbnm added 4 commits November 18, 2020 18:14

save

e3f89a2

Merge branch 'master' of github.com:pytorch/pytorch into view-dtype

24ab344

Merge branch 'master' of github.com:pytorch/pytorch into view-dtype

724cf0f

save

d2a4d3e

zasdfgbnm added 2 commits January 6, 2021 09:25

Merge branch 'master' of github.com:pytorch/pytorch into view-dtype

f849256

fix docs

cc88ec7

mruberry self-requested a review January 7, 2021 23:54

mruberry approved these changes Jan 7, 2021

View reviewed changes

ngimel reviewed Jan 8, 2021

View reviewed changes

zasdfgbnm added 2 commits January 7, 2021 16:58

save

5a4b1da

save

a3295fc

ngimel approved these changes Jan 8, 2021

View reviewed changes

facebook-github-bot reviewed Jan 8, 2021

View reviewed changes

zasdfgbnm mentioned this pull request Jan 8, 2021

Kill non-bool version of maskedXXX from TH and THC #50250

Closed

facebook-github-bot closed this in d00aceb Jan 8, 2021

facebook-github-bot added the Merged label Jan 8, 2021

zasdfgbnm deleted the view-dtype branch January 8, 2021 17:41

zasdfgbnm mentioned this pull request Feb 22, 2022

Add tensor.view(dtype) overload support csarofeen/pytorch#1481

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tensor.view(dtype) #47951

Add tensor.view(dtype) #47951

zasdfgbnm commented Nov 14, 2020

zasdfgbnm commented Nov 14, 2020

dr-ci bot commented Nov 14, 2020 •

edited by facebook-github-bot

Loading

zasdfgbnm Nov 14, 2020

ngimel Jan 8, 2021

zasdfgbnm Jan 8, 2021

vadimkantorov commented Nov 14, 2020

vadimkantorov commented Nov 14, 2020

zasdfgbnm Nov 15, 2020 •

edited

Loading

eellison Nov 18, 2020

zasdfgbnm Nov 19, 2020

codecov bot commented Nov 15, 2020 •

edited

Loading

ngimel commented Nov 18, 2020

mruberry commented Nov 18, 2020

eellison Nov 18, 2020 •

edited

Loading

zasdfgbnm Nov 18, 2020

facebook-github-bot left a comment

albanD Nov 18, 2020

albanD Nov 18, 2020 •

edited

Loading

zasdfgbnm Nov 19, 2020

albanD Nov 19, 2020

zasdfgbnm commented Jan 5, 2021

mruberry commented Jan 6, 2021

zasdfgbnm commented Jan 6, 2021

mruberry left a comment

ngimel Jan 8, 2021

zasdfgbnm Jan 8, 2021 •

edited

Loading

ngimel Jan 8, 2021

facebook-github-bot left a comment

facebook-github-bot commented Jan 8, 2021

Add tensor.view(dtype) #47951

Add tensor.view(dtype) #47951

Conversation

zasdfgbnm commented Nov 14, 2020

zasdfgbnm commented Nov 14, 2020

dr-ci bot commented Nov 14, 2020 • edited by facebook-github-bot Loading

💊 CI failures summary and remediations

🕵️ 1 new failure recognized by patterns

pytorch_windows_vs2019_py36_cuda10.1_test2 (1/1)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vadimkantorov commented Nov 14, 2020

vadimkantorov commented Nov 14, 2020

zasdfgbnm Nov 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 15, 2020 • edited Loading

Codecov Report

ngimel commented Nov 18, 2020

mruberry commented Nov 18, 2020

eellison Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

albanD Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zasdfgbnm commented Jan 5, 2021

mruberry commented Jan 6, 2021

zasdfgbnm commented Jan 6, 2021

mruberry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zasdfgbnm Jan 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Jan 8, 2021

dr-ci bot commented Nov 14, 2020 •

edited by facebook-github-bot

Loading

zasdfgbnm Nov 15, 2020 •

edited

Loading

codecov bot commented Nov 15, 2020 •

edited

Loading

eellison Nov 18, 2020 •

edited

Loading

albanD Nov 18, 2020 •

edited

Loading

zasdfgbnm Jan 8, 2021 •

edited

Loading