8000 expanded scalar is not contiguous · Issue #3919 · pytorch/pytorch · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

expanded scalar is not contiguous #3919

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
shekhovt opened this issue Nov 28, 2017 · 10 comments
Closed

expanded scalar is not contiguous #3919

shekhovt opened this issue Nov 28, 2017 · 10 comments

Comments

@shekhovt
Copy link

torch.Tensor(1,1).expand(1,2).view(1,2)

Traceback (most recent call last):
File "", line 1, in
RuntimeError: invalid argument 1: input is not contiguous at /pytorch/torch/lib/TH/generic/THTensor.c:231

There are two problems:

  1. view of the same size as the input should always succeed, regardless continuity
  2. an expanded scalar is of course contiguous, no?
    I think these are bugs.

As a feature request, may I suggest that some of the reshaping should also succeed on non-contiguous Tensors as long as only dimensions that are contiguous are affected. Examples:
size (4,4, X), strides (1,4, Y) -> size (16, X), stride (1, Y)
@gchanan
Copy link
Contributor
gchanan commented Nov 28, 2017

On 2., the docs could be a little cleaner; right now they say "Returns True if this tensor is contiguous in memory in C order," but it's not really clear (to me at least) what "in C order" means in the case of 0-strided (i.e. expanded) tensors. The condition is actually something like "contiguous in memory in C order and each point is non-aliased", "contiguous in memory in C with no strides of size 0" or "memory is equivalent to a contiguous C N-dimensional array of the corresponding size".

On 1., we probably need more "viewing" functions, i.e. a "reshape" (that is a no-op for input of the same size", a "view" that always succeeds, and maybe the current view (that requires contiguity).

@shekhovt
Copy link
Author

view is kind of attaching a new handle with some size and strides to the same data, without rearranging the data in memory. This is certainly possible in this case:
torch.Tensor(1,1).expand(1,2).view(-1)
by setting size=(2), strides=(0). Even though this is not contiguous in the sense of the existing definition, as you explain, it would be more complete and consistent if it covered such cases.

@vadimkantorov
Copy link
Contributor

proposed a similar thing to (1) in #3653

@shekhovt
Copy link
Author
shekhovt commented Nov 28, 2017 •
edited
Loading

I checked how it is in numpy, and they support all the mentioned cases, including zero strides and #3653.
In fact, one can hack around Tensor.view through numpy:

import torch
import numpy as np

a = torch.Tensor(16, 8, 32, 64).transpose(-1, -2)
#b = a.view(-1, 64, 32)  # does not work
b = torch.from_numpy(a.numpy().reshape([-1, 64, 32]))  # works

You can verify that a and b share the same memory and inspect

b.numpy().strides
(8192, 4, 256)

Only it does not work for cuda Tensors as the data pointer to device is barriered off numpy. But I hope I made my point that Tensor.view can work this way and it would be more clear and consistent.

@colesbury
Copy link
Member

An expanded Scalar is not C-contiguous in either PyTorch or NumPy.

>>> np.broadcast_to(np.array(1), (5, 5)).flags['C_CONTIGUOUS']
False

C-contiguous means that moving to the next element in the array moves one over in the underlying memory block. This is not the case for expanded scalars since. Having the memory block be contiguous in memory is not sufficient.

We should add reshape(). That's already discussed in other issues.

@shekhovt
Copy link
Author

Thanks, I understand C-contiguous. But is it the right prerequisite for view to work? It can work in all the following cases:
sizes are the same
reshaping a subsequence of dimensions with zero strides
reshaping a subsequence of C-contiguous dimensions

I believe they are working in numpy and it does not have to do with all mechanics in reshape:

a = np.broadcast_to(np.array(1), (2, 2)).view()
a.shape = [1,4]

array([[1, 1, 1, 1]])

@colesbury
Copy link
Member

Tensor.view is not equivalent to numpy.reshape (which does not require C-contiguity) nor ndarray.view (which changes data type)

@shekhovt
Copy link
Author

That I cannot argue about. I just suggest that there is a good deal of use cases where one wants a view of partially broadcasted tensors, where copying or changing data type can be avoided, and the current implementation is not supporting it

@colesbury
Copy link
Member

Yes, agreed. But we want to solve it by adding reshape() instead of changing view.

@shekhovt
Copy link
Author

In that case what would be the conditions that the new reshape() does not rearrange the data? And sorry, the .view() in the code above was indeed irrelevant.

This was referenced Dec 4, 2017
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@colesbury @vadimkantorov @shekhovt @gchanan
0