xtensor-stack / xtensor

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

xview strides correctness #566

Closed rayglover-ibm closed 6 years ago

rayglover-ibm commented 6 years ago

There appears to be a correctness problem with xview<CT, S...>::strides() in the following scenario:

xt::xtensor<float, 2> A{
    {1, 2},
    {3, 4},
    {5, 6}
};
auto col = xt::view(A, xt::all(), 1);
auto row = xt::view(A, 1, xt::all());

std::cout
    << "A: " << A.strides()[0] << ' ' << A.strides()[1] << '\n'
    << "col: " << col.strides()[0] << ' ' << col.strides()[1] << '\n'
    << "row: " << row.strides()[0] << ' ' << row.strides()[1] << '\n';

Produces:

A: 2 1
col: 2 4
row: 1 4

Where does the 4 come from? If I look at the algorithm, the line here is using idx, the value of which is out-of-bounds for the strides type in my example.

JohanMabille commented 6 years ago

col and row are 1-dimensional views, so col.strides().size() and row.strides().size() should be 1, and the 4 is just an invalid (past-the-end) value.

Can you confirm the size of the strides of row and col ?

rayglover-ibm commented 6 years ago

That's possibly the issue then, the type is not the correct size (although strides_type is):

std::cout << typeid(decltype(row)::strides_type).name() << '\n';
std::cout << typeid(row.strides()).name() << '\n';

Produces (with c++filt -t):

std::array<unsigned long, 1ul>
std::array<unsigned long, 2ul>
JohanMabille commented 6 years ago

Indeed the return type of the strides function is wrong. Thanks for catching it!