xtensor-stack / xtensor

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

Resize semantic #1445

Open PerretB opened 5 years ago

PerretB commented 5 years ago

The resize function currently discards all container values (except if the expected size is equal to the current size): this is a major difference with the STL containers behavior.

Example:

xtensor<int, 1> a{0, 1, 2, 3, 4};
a.resize({4});
std::cout << a; // undefined behavior

IMHO an STL compliant API which limits surprises would be better (perhaps with an extra option to disable data copying). This should also be described in the documentation (I maybe missed it).

SylvainCorlay commented 5 years ago

Thanks for opening the issue. Just adding a quick note that reshape preserves data.

wolfv commented 5 years ago

This is also not trivial to implement. A resize would change the position of all data if done trivially, and otherwise would be very costly because memory needs to be copied into non-contigous regions.

JohanMabille commented 5 years ago

We can implement it as an option of resize, as it is done in uBlas where resize accepts an additional preserve boolean parameter.

PerretB commented 5 years ago

I think that the solution proposed by Johan would be nice. In particular, it removes the ambiguity on the expected behavior of the function: this was my main complaint as the resize with preserve is indeed easily obtained with views.

tdegeus commented 5 years ago

I was just browsing through the new quickref section of the docs. With the current implementation a remark in the resize section might be in order?

JohanMabille commented 5 years ago

Indeed, that should be stated until the preserve feature is implemented.

JohanMabille commented 5 years ago

The documentation has been updated in #1447

mokenyi commented 4 years ago

Hi,

Would this be a valid implementation of a value-preserving resize for xtensors? I suspect that it's not as general as the design of your library would allow, but at the moment I just need something quick, dirty and hopefully bug-free.

  template<typename T, size_t N>
  void conservative_resize(xt::xtensor<T,N>& x, const std::vector<size_t> sh)
  {
    int c(0);

    xt::xarray<T> tmp=xt::zeros<T>(sh);
    xt::xstrided_slice_vector sv;
    for (size_t s: x.shape())
      sv.push_back(xt::range(0,s));

    xt::strided_view(tmp, sv)=x;
    x.resize(sh);
    x=tmp;
  }
JohanMabille commented 4 years ago

I think the behavior of this implementation is correct if and only if sh.size() == x.size().

jonathonl commented 3 years ago

I just want to point out that the OP example does not necessarily require any data copying. By altering https://github.com/xtensor-stack/xtensor/blob/a8833f07483de4a940a9093960e7372e272f7b97/include/xtensor/xstorage.hpp#L222-L233 to check if new_size is greater than old_size before reallocating memory, original values would be retained when shrinking the container size.

template <class T, class A>
inline void uvector<T, A>::resize_impl(size_type new_size)
{
    size_type old_size = size();
    pointer old_begin = p_begin;
    if (new_size != old_size)
    {
        if (new_size > old_size)
        {
            p_begin = detail::safe_init_allocate(m_allocator, new_size);
            detail::safe_destroy_deallocate(m_allocator, old_begin, old_size);
        }
        p_end = p_begin + new_size;
    }
}

This would only work if the first dimension (for row-major) is the only one that is changing in size. For example, this would work for removing rows from the end of a matrix without a reallocation.

xtensor<int, 2> a{
  {0, 1}, 
  {2, 3}, 
  {4, 5}};
a.resize({2, 2});