xtensor-stack / xtensor

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

Bug with reducing over multiple axes when one is length 0 #1563

Closed DavisVaughan closed 5 years ago

DavisVaughan commented 5 years ago

I have gathered from playing with numpy that reducing functions have the property that, no matter what, when you reduce over an axis the size of that axis becomes 1. This applies even if the axis was originally a 0.

With that in mind, here are examples of performing a reducing sum over a (0, 1) shaped input. I am using keep_dims solely for the purpose of helping me understand where the reductions happen. The first two do what I expect.

But for the last one we reduce over all axes {0, 1}. The shape looks correct, (1, 1), but if you try and print the result R crashes.

// [[Rcpp::depends(xtensor)]]
// [[Rcpp::plugins(cpp14)]]

#include <xtensor/xarray.hpp>
#include <xtensor-r/rarray.hpp>
#include <xtensor/xio.hpp>

// [[Rcpp::export]]
void test_sum_axis_0() {
  xt::xarray<int>::shape_type shp = {0, 1};
  xt::xarray<int> x(shp);

  auto res = xt::sum(x, 0, xt::keep_dims);

  std::cout << "res " << std::endl << res << std::endl;;
  std::cout << "res shape (" << res.shape()[0] << ", " << res.shape()[1] << ")" << std::endl;
}

// [[Rcpp::export]]
void test_sum_axis_1() {
  xt::xarray<int>::shape_type shp = {0, 1};
  xt::xarray<int> x(shp);

  auto res = xt::sum(x, 1, xt::keep_dims);

  std::cout << "res " << std::endl << res << std::endl;;
  std::cout << "res shape (" << res.shape()[0] << ", " << res.shape()[1] << ")" << std::endl;
}

// [[Rcpp::export]]
void test_sum_axis_all() {
  xt::xarray<int>::shape_type shp = {0, 1};
  xt::xarray<int> x(shp);

  auto res = xt::sum(x, {0, 1}, xt::keep_dims);

  //std::cout << "res " << std::endl << res << std::endl;;
  std::cout << "res shape (" << res.shape()[0] << ", " << res.shape()[1] << ")" << std::endl;
}
Rcpp::sourceCpp("~/Desktop/test.cpp")

# looks good
test_sum_axis_0()
#> res 
#> {{0}}
#> res shape (1, 1)

# looks good
test_sum_axis_1()
#> res 
#> {}
#> res shape (0, 1)

# the shape looks right, but it crashes
# if you print `res`
# i expect the answer to be {{0}} 
test_sum_axis_all()
#> res shape (1, 1)

FWIW, numpy returns what I expect here, which is {{0}} with a shape of (1, 1)

SylvainCorlay commented 5 years ago

It looks like a lot of this relates to arrays that have 0-length in some direction.

DavisVaughan commented 5 years ago

This might be a slightly different problem than #1562 but I am not sure.

JohanMabille commented 5 years ago

@DavisVaughan I cannot reproduce with last master, I think this bug has been fixed in #1582. Can you confirm?

DavisVaughan commented 5 years ago

This exact code doesn't crash for you?

I am currently on the master version of everything, and can confirm that #1582 did fix #1562, but I don't think it fixed this.

(Note that I had commented out the problematic line in test_sum_axis_all() above)

// [[Rcpp::depends(xtensor)]]
// [[Rcpp::plugins(cpp14)]]

#include <xtensor/xarray.hpp>
#include <xtensor-r/rarray.hpp>
#include <xtensor/xio.hpp>

// [[Rcpp::export]]
void test_sum_axis_all() {
  xt::xarray<int>::shape_type shp = {0, 1};
  xt::xarray<int> x(shp);

  auto res = xt::sum(x, {0, 1}, xt::keep_dims);

  std::cout << "res " << std::endl << res << std::endl;;
}
JohanMabille commented 5 years ago

My bad, I didn't run the correct test. Indeed I have a crash.