Channels last stored in tensor #23391
Conversation
pytorchbot
added
module: internals
VitalyFedyunin
force-pushed
the
channels_last_stored_in_tensor
branch
from
baf6bf6 to
9454dd5
Compare
| if (dim() == 4) { | ||
| int64_t expected = 1; | ||
| for (auto& d : {1, 3, 2, 0}) { | ||
| if (size(d) != 1) { |
There was a problem hiding this comment.
why skip for size 1? better to be safer and check it.
Also, there might be special effects if H=W=1 but I guess it's less likely.
There was a problem hiding this comment.
Because size 1 strides is broken in PyTorch, for example:
N = 10; C = 3; H = 32; W = 16;
x = torch.randn(N,H,W,C)
x = x.permute(0,3,1,2)
y = x[0]
z = y.unsqueeze(0)
y.stride()
Out[40]: (1, 48, 3)
In [41]: z.stride()
Out[41]: (3, 1, 48, 3)There was a problem hiding this comment.
I don't follow the reasoning here. There doesn't seem to be anything wrong with x[0], so you must be complaining about the behavior of unsqueeze. But the stride of a size one dimension doesn't actually matter, since it doesn't ever get used. Maybe the choice of the last element is a bit goofy; we probably did it because it's compatible with Numpy's behavior:
>>> N = 10; C = 3; H = 32; W = 16
>>> x = np.zeros((N,H,W,C))
>>> x = x.transpose(0,3,1,2)
>>> y = x[0]
>>> y.strides
(8, 384, 24)
>>> np.expand_dims(y, 0).strides
(24, 8, 384, 24)
There was a problem hiding this comment.
Maybe your claim is that, since strides don't matter for size one dimensions, we should ignore them. I can get behind that reason. But I'm not sure I would jump to the conclusion that it is "broken".
There was a problem hiding this comment.
In any case this should be amply documented :>
There was a problem hiding this comment.
Hmm, you should probably also skip zero size dimensions, following this reasoning.
There was a problem hiding this comment.
Agree, broken is incorrect word here, what I meant is they carry zero information.
| return false; | ||
| } | ||
|
|
||
| bool TensorImpl::compute_strides_like_channels_last() const { |
There was a problem hiding this comment.
are you trying to cover slicing of channel-last tensors? does it actually occur in practice?
There was a problem hiding this comment.
Slicing and bounding. Both might happen in pre-processing.
VitalyFedyunin
changed the title
| TensorTypeId type_id_; | ||
| bool is_contiguous_ = true; | ||
| bool is_channels_last_contiguous_ = false; | ||
| bool is_channels_last_ = false; |
There was a problem hiding this comment.
Would it be possible to add a comment explaining the difference between is_channels_last_contiguous_ and is_channels_last_ here? Based on the rest of the patch, I think I know what it means, but I don't think it's obvious!
| return impl_->is_contiguous(memory_format); | ||
| } | ||
|
|
||
| at::MemoryFormat suggest_memory_format() const { |
There was a problem hiding this comment.
Dead code. When is it going to be used?
There was a problem hiding this comment.
In follow up PRs by me, Igor and Jie.
ezyang
left a comment
ezyang
left a comment
There was a problem hiding this comment.
I'd like more docs but the substantive logic makes sense.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@VitalyFedyunin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@VitalyFedyunin merged this pull request in 6e4a83a. |
Summary: Define 4D tensor as stored in channels last memory format, when dimensions order is NCHW and C-strides < W-strides < H-strides < N-strides (If size of any dimension is equal to 1, this dimension strides value is not taken into account). Channels last contiguous tensor is channel last tensor which occupies contiguous memory block. So x.is_contiguous(memory_format=torch.channels_last) checks if tensor is channels last contiguous. Pull Request resolved: pytorch/pytorch#23391 Differential Revision: D16601414 Pulled By: VitalyFedyunin fbshipit-source-id: 8d098e7eec2f00fb1d12261bc240b3645d4f5b73
6 participants
Define 4D tensor as stored in channels last memory format, when dimensions order is NCHW and C-strides < W-strides < H-strides < N-strides (If size of any dimension is equal to 1, this dimension strides value is not taken into account).
Channels last contiguous tensor is channel last tensor which occupies contiguous memory block. So x.is_contiguous(memory_format=torch.channels_last) checks if tensor is channels last contiguous.