xtensor-stack / xtensor-r

R bindings for xtensor
BSD 3-Clause "New" or "Revised" License
87 stars 15 forks source link

Bug with reducers and 1D arrays #74

Closed DavisVaughan closed 5 years ago

DavisVaughan commented 5 years ago

Hi team, I'm working through some reducer examples, and think there might be a bug with rarrays and reducers. First off, here is a cpp script you can hopefully use to reproduce the issue.

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

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

#include <Rcpp.h>
using namespace Rcpp;

// [[Rcpp::export]]
double rarray_sum_cpp() {
  xt::rarray<double> x = {1, 2, 3, 4};
  xt::rarray<double> res = xt::sum(x, 0);

  //Crash
  //std::cout << res << std::endl;

  // NULL?
  SEXP r_res = res;
  Rcpp::print(r_res);

  return(1);
}

// [[Rcpp::export]]
double xtensor_sum_cpp() {
  xt::xarray<double> x = {1, 2, 3, 4};
  const xt::xarray<double>& res = xt::sum(x, 0);
  std::cout << res << std::endl;
  return(1);
}

Running xtensor_sum_cpp() is fine. We see the 10 result.

> xtensor_sum_cpp()
[1] 1
 10.

Running rarray_sum_cpp() is...problematic. I can't try to do the same example and print out res. That crashes for me. What I can do is convert res to SEXP and try to print that out. Strangely I get NULL.

> rarray_sum_cpp()
NULL
[1] 1

I'm allowing 1D arrays in rray again, so this would be an issue (previously i removed them).

Any ideas?

DavisVaughan commented 5 years ago

I should also add that it looks like matrix and higher dimension examples are working, so its just a 1D thing.

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

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

#include <Rcpp.h>
using namespace Rcpp;

// [[Rcpp::export]]
xt::rarray<double> rarray_matrix_sum_cpp() {

  xt::rarray<double> x = {{1, 2}, {3, 4}};
  xt::rarray<double> res = xt::sum(x, 0);

  std::cout << res << std::endl;

  return(res);
}
> rarray_matrix_sum_cpp()
{ 4.,  6.}
[1] 4 6
SylvainCorlay commented 5 years ago

On the top of my head, I think that the issue is that R does not really have a notion of 0-D array.

xtensor reducers in fact return an expression of dimension Original dimension - Number of reducing axes which is 0-D when reducing over the only dimension of a 1-D array.

Maybe a way to work around this is to check if the assigned expression is 0-D and return a scalar to R in that case...

DavisVaughan commented 5 years ago

This can probably be closed b/c of pr #77

SylvainCorlay commented 5 years ago

Thanks!