Figure out and fix Tensor(Storage) constructor

[pytorch]

Figure out and fix Tensor(Storage) constructor

[apaszke]

Yeah, keyword args are a reasonable solution, but I would require them only for long tensors (and optionally allow them for other types).

[colesbury]

@apaszke, my point is that we shouldn't have some constructors work for only some tensor types. That leads to non-obvious bugs because we write a lot of code that is generic in the type of the tensor, especially in nn. The convenience of sometimes not having to use a keyword argument is not worth the inconsistency.

So to be clear, I don't think we should make any arguments optional for only some tensor types.

[colesbury]

I got bitten by this again in the PR #127 -- it breaks with LongTensors.

I have a new proposal that avoids requiring keyword arguments:

We change Tensor.size() to return a new type torch.Size() instead of torch.LongStorage. The torch.Size() type provides a some standard operations like indexing, but it's immutable. A tensor can be constructed a torch.Size() type so the following still works:

foo = torch.FloatTensor(tensor.size())

Some more examples:

>>> torch.FloatTensor(3, 4, 2).size()
torch.Size([3, 4, 2])
>>> torch.FloatTensor(3, 4, 2).size()[0]
3
>>> torch.FloatTensor(torch.Size([3, 4, 2]))
...
[torch.FloatTensor of size 3x4x2] 
>>> torch.LongTensor(torch.LongStorage([1, 2, 3])) # as opposed to a LongTensor of size 1x2x3
 1
 2
 3
[torch.LongTensor of size 3]

If you guys are OK with this, I'll make the change.

cc @apaszke, @soumith, @adamlerer, @fmassa

[soumith]

i think this is way more reasonable. Returning torch.LongStorage never made sense anyways.

[apaszke]

This seems to be an elegant solution. It'd be probably the best to make torch.Size a subclass of tuple.

class Size(tuple):
    pass

Btw, if you have a regular tuple or list of sizes, you can do torch.Tensor(*sizes), instead of torch.Tensor(torch.Size(sizes)).

[colesbury]

@adamlerer was advocating for making Tensor.size() simply return a tuple (as opposed to a sublcass of tuple).

To construct a tensor with the same size as another, you would write:

tensor1 = torch.Tensor(*tensor2.size())

This seems reasonable to me too, but would require more changes to existing code.

[apaszke]

I thought about it as well, but I dislike that it requires an additional star. I'd say that size() should return a special Size object that behaves just like a regular tuple (because it subclasses it), but has a clear meaning in our functions. If we make it just a regular tuple I can see a lot of issues where people will forget about the star and will be very surprised that they got e.g. a 3 element tensor.

[colesbury]

I think this is fixed now