flambeau icon indicating copy to clipboard operation
flambeau copied to clipboard

[RFC] Should we keep dimensions (like Arraymancer) or squezze dimensions (like Pytorch) by default

Open HugoGranstrom opened this issue 3 years ago • 5 comments

When we for example slice a Tensor or perform a reduction along an axis we have two options: either to keep the dimensions of the original Tensor even if its size along that axis is 1. Or we can squeeze away that extra dimension. For example, let's say we have this Tensor:

1 2
3 4
Tensor[2, 2]

If we slice it like this: t[_, 0] should we get back a tensor like

1
3
Tensor[2, 1] # Arraymancer

or

1 3
Tensor[2] # Pytorch

I vouch for going against libtorch's defaults and use the Arraymancer way. The reason for this is that it is trivial to squeeze away the unnecessary dimensions with t.squezze while it is non-trivial to restore that extra dimension(s).

HugoGranstrom avatar Jan 23 '21 14:01 HugoGranstrom

Note that Arraymancer provides at to have dimension coalescing in a less verbose way. t.at(_, 0)

LibTorch provides keepdim parameter for every native LibTorch function which default to false. The [] in flambeau is maps to keepdim = false.

Personally I think it's best if we follow PyTorch/Torch to ease porting existing programs by hopefully copy-paste.

mratsim avatar Jan 23 '21 14:01 mratsim

That's a fair point! 👍 But it wouldn't hurt to have a similar version here but the other way around? So that t.at() sets keepDim=true?

HugoGranstrom avatar Jan 23 '21 15:01 HugoGranstrom

I agree. Changing Torch's convention will break expectation and make the bindings "less intuitive" when compared to existing Torch code.

Clonkk avatar Jan 23 '21 15:01 Clonkk

My mind is already in the high-level wrapper it seems 🙃

HugoGranstrom avatar Jan 23 '21 15:01 HugoGranstrom

I think for Flambeau, having it match PyTorch's behavior would be better for portability of code and to make the transition easier for users coming from PyTorch. So to squeeze the dimension: t[_, 0], and to keep the dimension: t[_, 0..0]. I also find this intuitive since integer indexes also do dimension reduction on Nim arrays of arrays and seqs of seqs. And since slices on tensors have to keep the dimension if the slice's range is greater than a single value, they naturally signal dimension preservation. So for me, these two ways of indexing seem like a nice way of differentiating the two different keepdim options, that integer indexes result in dimension reduction and that slices result in dimension preservation.

elliotwaite avatar Jan 24 '21 07:01 elliotwaite