xtensor-stack / xtensor

C++ tensors with broadcasting and lazy computing
BSD 3-Clause "New" or "Revised" License
3.35k stars 398 forks source link

Behavior of operator[](size_t) #2380

Open JohanMabille opened 3 years ago

JohanMabille commented 3 years ago

Today, operator[](size_t) behaves like operator(). For expression with dimension > 2, this means the argument of operator is not a flat index into the underlying storage (whether it exists or is a simple abstraction) but the index in the last dimension.

Some people reported issues regarding that behavior, because they were expecting the argument of operator[] to be the flat index into the underlying storage.

So basically we have two options:

Rigorously, one could argue that if operator()(i, j, k) is equivalent to operator[]({i, j, k}), then operator()(i) should be equivalent to operator[]({i}) and there is not really any objection to have operator[] equivalent to flat.

Notice that for row-major containers, this change would be transparent for the user (the only difference being that some failing bounds checks will pass after the change).

cc @SylvainCorlay @tdegeus @wolfv @adriendelsalle @gouarin

tdegeus commented 3 years ago

My personal preference in to have operator[](size_t) == flat(size_t) for the following reason. I from time implement functions that can accept arrays but themselves operate only on the flat storage (i.e. fine, the input was an array, but I just operate as if it was a vector). For this case, have operator[](size_t) == flat(size_t) would allow me to have an implementation that can accept std::vector as well as xt::xarray templates, which I would find quite nice. Currently this is only possible if the array has dimension one only. Beyond this, I have quite some functions that act on the flat storage, and I personally would prefer the shorter a[i] than a.flat(i) (or the even worse current solution a.data()[i]). But ok, this is style and not really an argument.

My second best would have to operator[](size_t) throw in dimension > 1. The current behaviour of prepending n = dimension() - 1 zeros is quite different than NumPy and in my opinion quite marginal.

Instead, prepending the index with zeros could be generalised and have its own operator, see proposition here https://github.com/xtensor-stack/xtensor/pull/2383. Note that I find having such operators nice regardless of operator[](size_t) acts.