Adding memory_format to empty and empty_like operators #20558

VitalyFedyunin · 2019-05-15T22:28:41Z

Original RFC #19092

To ensure that we are not introducing BC breaking change, empty_like returns contiguous tensor by default.

nCwh = torch.randn(N, C, H, W)
nhwC = nCwh.contiguous(memory_format=torch.channels_last)

new_nCwh = torch.empty_like(nhwC) 
new_nCwh.is_contiguous(memory_format=torch.channels_last) == False

Now we need a way to preserve memory format in empty_like

nCwh = torch.randn(N, C, H, W)
nhwC = nCwh.contiguous(memory_format=torch.channels_last)

new_nhwC = torch.empty_like(nhwC, memory_format=torch.preserve_format)
new_nhwC.is_contiguous(memory_format=torch.channels_last) == True

like_nCwh = torch.empty_like(nCwh, memory_format=torch.preserve_format)
like_nCwh.is_contiguous(memory_format=torch.channels_last) == False

Usage of torch.preserve_format allows us to avoid if constructs.

We can also generate different memory format outputs

nCwh = torch.randn(N, C, H, W)
nhwC = nCwh.contiguous(memory_format=torch.channels_last)

new_nhwC = torch.empty_like(nCwh, memory_format=torch.channels_last)
new_nhwC.is_contiguous(memory_format=torch.channels_last) == True

new_nCwh = torch.empty_like(nhwC, memory_format=torch.contiguous_format)
new_nCwh.is_contiguous(memory_format=torch.channels_last) == False

aten/src/ATen/native/TensorFactories.cpp

VitalyFedyunin · 2019-05-22T19:04:31Z

To ensure that we are not introducing BC breaking change, empty_like returns contiguous tensor by default.

nCwh = torch.randn(N, C, H, W)
nhwC = nCwh.contiguous(memory_format=torch.channels_last)

new_nCwh = torch.empty_like(nhwC) 
new_nCwh.is_contiguous(memory_format=torch.channels_last) == False

Now we need a way to preserve memory format in empty_like

nCwh = torch.randn(N, C, H, W)
nhwC = nCwh.contiguous(memory_format=torch.channels_last)

new_nhwC = torch.empty_like(nhwC, memory_format=torch.preserve_format)
new_nhwC.is_contiguous(memory_format=torch.channels_last) == True

like_nCwh = torch.empty_like(nCwh, memory_format=torch.preserve_format)
like_nCwh.is_contiguous(memory_format=torch.channels_last) == False

Usage of torch.preserve_format allows us to avoid if constructs.

We can also generate different memory format outputs

nCwh = torch.randn(N, C, H, W)
nhwC = nCwh.contiguous(memory_format=torch.channels_last)

new_nhwC = torch.empty_like(nCwh, memory_format=torch.channels_last)
new_nhwC.is_contiguous(memory_format=torch.channels_last) == True

new_nCwh = torch.empty_like(nhwC, memory_format=torch.contiguous)
new_nCwh.is_contiguous(memory_format=torch.channels_last) == False

dzhulgakov · 2019-05-24T06:25:25Z

c10/core/TensorImpl.cpp

-      }
-    }
-    return false;
+      return dim() == 4 && strides() == get_channels_last_strides(sizes());


is C++ smart enough to inline std::vector and avoid memory allocation? if not, I wouldn't pull out this function

This function call will go away in the next PR (Where I introduce 'tag') as calculating this every time is very inefficient.

Is the tag advisory or not? If the tag is advisory, I would not expect is_contiguous to change behavior depending on if the tag is set or not (and so I agree with @dzhulgakov here). If the tag is non-advisory, I have concerns about the overall design :)

Since we killed tags, now this comment applies once again, right?

This code line will go away anyway in non 'tag' option too.

For peace of mind, I will not pull out it now. And replace copy-pasted code in the next PR.

c10/core/TensorImpl.h

…ry_format_empty_like_v2

facebook-github-bot

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

VitalyFedyunin · 2019-06-13T21:12:52Z

@ailzhang Looks like it breaks XLA, what is the course of the actions to patch XLA?

…_like

ailzhang

At the first glance it looks easy to fix on xla side. So I'm fine with landing this as long as others are happy about the change.
(in the worst case I might need to revert :P )

VitalyFedyunin · 2019-06-19T02:19:31Z

@pytorchbot retest this please

tools/jit/gen_jit_dispatch.py

ezyang · 2019-06-19T13:59:42Z

aten/src/ATen/native/TensorFactories.cpp

    result.sparse_resize_and_clear_(size, size.size(), 0);
  } else {
    result.resize_(size);
+    auto memory_format = optional_memory_format.value_or(MemoryFormat::Contiguous);


This is guaranteed to be none, right? So just set memory_format = Contiguous

ezyang · 2019-06-19T15:13:18Z

aten/src/ATen/native/TensorFactories.cpp

+    } else if (self.is_contiguous(MemoryFormat::Contiguous)) {
+      use_memory_format = MemoryFormat::Contiguous;
+    } else {
+      TORCH_CHECK(


Some wild speculation. Since we've completely removed the memory tag, what if we ALWAYS preserved striding in the preserve case? (Instead of only preserving if we matched a memory format). This would subsume all of the checking here, and it feels more morally correct.

This doesn't have to be fixed today because the error gives us an out; but it seems worth thinking about.

I have been thinking same way, as it kind of makes sense to return channels_last tensor if I do something like:

y = torch.empty_like( nhwc[0].unsqueeze(0) )

ezyang · 2019-06-19T15:18:47Z

aten/src/ATen/native/TensorProperties.cpp

-  }
+  TORCH_CHECK(
+      memory_format != MemoryFormat::Preserve,
+      "preserve memory format is unsupported by the contiguous operator");


Or we could just no-op if you pass preserve to contiguous :) (I'm not sure if that makes any code easier to write lol)

no-op meaning return itself? It is kind of unclear what to do with randomly permuted tensor in this case. I rather throw error instead of guessing what user mean.

ezyang · 2019-06-19T15:30:55Z

test/test_torch.py

+    def _test_memory_format_empty_like(self, x):
+        nhwc = x.contiguous(memory_format=torch.channels_last)
+
+        like = torch.empty_like(nhwc, memory_format=torch.preserve_format)


I feel that these tests are not sufficient. Suppose I write a buggy implementation of the stride computation so that I put channels before batch rather than after. I don't think any of these tests would catch an error like that.

There is some small utility in replicating the stride logic for a specific dimensionality here, because when you generalize the formula it will help catch errors in a less correlated way. Other properties you could check are that transpositions related the striding between channels first and channels last.

Agree, but it will make sense as part of the next PR where I plan to introduce this channels_last criteria, and cover basic operations on it.

ezyang · 2019-06-19T15:32:13Z

In the interest of being super explicit, here is what I would like to see before landing:

Comment on empty_out explaining why it takes optional_memory_format and then immediately asserts it must be nullopt
Fix for https://github.com/pytorch/pytorch/pull/20558/files#r295312972
Fix for https://github.com/pytorch/pytorch/pull/20558/files#r287228976

Things that I'd like to see soon:

Generalization of stride checking to arbitrary dimensionality
Improved testing https://github.com/pytorch/pytorch/pull/20558/files#r295363683

ezyang

See comments

…_like

VitalyFedyunin · 2019-06-20T18:36:12Z

Comment on empty_out explaining why it takes optional_memory_format and then immediately asserts it must be nullopt

Fix for https://github.com/pytorch/pytorch/pull/20558/files#r295312972

Fix for https://github.com/pytorch/pytorch/pull/20558/files#r287228976

Done

Generalization of stride checking to arbitrary dimensionality

Improved testing https://github.com/pytorch/pytorch/pull/20558/files#r295363683

Next smaller PR.

facebook-github-bot

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

…_like

facebook-github-bot

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

VitalyFedyunin · 2019-06-25T17:34:05Z

xla fail is expected, empty, empty_like and empty_out signatures have to be updated in xla

facebook-github-bot · 2019-06-26T20:36:19Z

@VitalyFedyunin merged this pull request in 516c7e4.

Summary: Original RFC pytorch/pytorch#19092 To ensure that we are not introducing BC breaking change, empty_like returns contiguous tensor by default. ```python nCwh = torch.randn(N, C, H, W) nhwC = nCwh.contiguous(memory_format=torch.channels_last) new_nCwh = torch.empty_like(nhwC) new_nCwh.is_contiguous(memory_format=torch.channels_last) == False ``` Now we need a way to preserve memory format in `empty_like` ```python nCwh = torch.randn(N, C, H, W) nhwC = nCwh.contiguous(memory_format=torch.channels_last) new_nhwC = torch.empty_like(nhwC, memory_format=torch.preserve_format) new_nhwC.is_contiguous(memory_format=torch.channels_last) == True like_nCwh = torch.empty_like(nCwh, memory_format=torch.preserve_format) like_nCwh.is_contiguous(memory_format=torch.channels_last) == False ``` Usage of `torch.preserve_format` allows us to avoid `if` constructs. We can also generate different memory format outputs ```python nCwh = torch.randn(N, C, H, W) nhwC = nCwh.contiguous(memory_format=torch.channels_last) new_nhwC = torch.empty_like(nCwh, memory_format=torch.channels_last) new_nhwC.is_contiguous(memory_format=torch.channels_last) == True new_nCwh = torch.empty_like(nhwC, memory_format=torch.contiguous_format) new_nCwh.is_contiguous(memory_format=torch.channels_last) == False ``` Pull Request resolved: pytorch/pytorch#20558 Differential Revision: D15502474 Pulled By: VitalyFedyunin fbshipit-source-id: 2e120d57eefad6fb8e04b8322c79871392f64331

gchanan reviewed May 16, 2019

View reviewed changes

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

VitalyFedyunin changed the title ~~[WIP] Adding memory_format to randn and empty operators~~ [WIP] Adding memory_format to empty and empty_like operators May 22, 2019

VitalyFedyunin mentioned this pull request May 22, 2019

Memory format support for contiguous and is_contiguous #20455

Closed

pytorchbot added the module: autograd Related to torch.autograd, and the autograd engine in general label May 22, 2019

VitalyFedyunin changed the title ~~[WIP] Adding memory_format to empty and empty_like operators~~ Adding memory_format to empty and empty_like operators May 22, 2019

VitalyFedyunin requested a review from dzhulgakov May 22, 2019 21:04

Add memory_format support to empty, empty_like and empty_out

fd52f83

VitalyFedyunin force-pushed the memory_format_empty_like branch from 4f347b2 to fd52f83 Compare May 22, 2019 21:08

VitalyFedyunin added 2 commits May 23, 2019 11:01

Fixing CPP test

fc2c68b

Trying to fix extentions test

9c4dbb9

pytorchbot added the module: cpp-extensions Related to torch.utils.cpp_extension label May 23, 2019

VitalyFedyunin added 2 commits May 23, 2019 12:29

Fix typo

675f0a7

Fixing JIT and tests

2749803

dzhulgakov reviewed May 24, 2019

View reviewed changes

VitalyFedyunin added 5 commits May 24, 2019 08:26

Merge branch 'master' of https://github.com/pytorch/pytorch into memo…

aef2fdb

…ry_format_empty_like_v2

Fix typehints

be6d829

Merge branch 'master' of https://github.com/pytorch/pytorch into memo…

8dcb9ef

…ry_format_empty_like_v2

Merge branch 'master' of https://github.com/pytorch/pytorch into memo…

03d1dfb

…ry_format_empty_like_v2

Fix TestTypeHints

f97de65

facebook-github-bot reviewed May 24, 2019

View reviewed changes

Triger [xla ci] tests

9d15490

Merge remote-tracking branch 'origin/master' into memory_format_empty…

c4c1681

…_like

VitalyFedyunin requested a review from ezyang June 18, 2019 20:39

ailzhang reviewed Jun 18, 2019

View reviewed changes

ezyang reviewed Jun 19, 2019

View reviewed changes

tools/jit/gen_jit_dispatch.py Show resolved Hide resolved

ezyang reviewed Jun 19, 2019

View reviewed changes

ezyang requested changes Jun 19, 2019

View reviewed changes

VitalyFedyunin added 2 commits June 20, 2019 11:21

Merge remote-tracking branch 'origin/master' into memory_format_empty…

367a7b6

…_like

Address feedback

5224a6a

facebook-github-bot reviewed Jun 20, 2019

View reviewed changes

VitalyFedyunin added 3 commits June 20, 2019 12:16

Fix merge

9fc3fef

Merge remote-tracking branch 'origin/master' into memory_format_empty…

07b7df3

…_like

Fix extension

909b908

facebook-github-bot reviewed Jun 25, 2019

View reviewed changes

ezyang approved these changes Jun 26, 2019

View reviewed changes

facebook-github-bot closed this in 516c7e4 Jun 26, 2019

facebook-github-bot added the merged label Jun 26, 2019

ailzhang mentioned this pull request Jun 27, 2019

Fix breakage 0626 pytorch/xla#785

Closed

mruberry added the Merged label Oct 28, 2020

Adding memory_format to empty and empty_like operators #20558

Adding memory_format to empty and empty_like operators #20558

Uh oh!

Conversation

VitalyFedyunin commented May 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

VitalyFedyunin commented May 22, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

VitalyFedyunin commented Jun 13, 2019

Uh oh!

ailzhang left a comment

Choose a reason for hiding this comment

Uh oh!

VitalyFedyunin commented Jun 19, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ezyang commented Jun 19, 2019

Uh oh!

ezyang left a comment

Choose a reason for hiding this comment

Uh oh!

VitalyFedyunin commented Jun 20, 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!

VitalyFedyunin commented Jun 25, 2019

Uh oh!

facebook-github-bot commented Jun 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

VitalyFedyunin commented May 15, 2019 •

edited

Loading