xtensor-stack / xtensor

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

Transpose of a 1*4 xarray resulting incorrect value #389

Closed marty1885 closed 7 years ago

marty1885 commented 7 years ago

Hello. I found a bug in xtensor when I'm writhing my own NN library in C++. The following code should make x a 4*1 array with value {{0},{1},{1},{1}}

xt::xarray<float> x = {{0,1,1,1}};
x = xt::transpose(x);
cout << x << endl;

However, it prints

{{ 0.},
 { 1.},
 { 1.},
 { 0.}}

Also, modifying the code to the following does not make the problem disappear.

xt::xarray<float> tmp = {{0,1,1,1}};
xt::xarray<float> x = xt::transpose(tmp);

But somehow making the type of x to be auto fixes it.

xt::xarray<float> tmp = {{0,1,1,1}};
auto x = xt::transpose(tmp);

Prints:
{{ 0.},
 { 1.},
 { 1.},
 { 1.}}

Making the above(working) code a one-liner also gives the correct answer.

auto x = xt::transpose(xt::xarray<float>({{0,1,1,1}}));

Yet, making it two lines results in incorrect value again

auto x = xt::xarray<float>({{0,1,1,1}});
x = xt::transpose(x);

Here is some code that demonstrates this problem. (It prints weird value like 0.000000e+00 if multiple of them are put in the same program.)

Tested on Arch Linux with the latest xtensor(cloned from github), GCC 7.1/clang 4.0. Compile with the flag -O0 (all optimization off)


Edit: Some further testing shows more weird behavior. Assigning a correct xarray to another one make it wrong.

auto x = xt::transpose(xt::xarray<float>({{0,1,1,1}}));
xt::xarray<float> y = x;

Prints:
{{ 0.},
 { 1.},
 { 1.},
 { 0.}}

And the working one-liner only works if x is declared as auto. Making it an xarray shows incorrect value.

xt::xarray<float> x = xt::transpose(xt::xarray<float>({{0,1,1,1}}));
SylvainCorlay commented 7 years ago

Thanks @marty1885 for the report.

This seems to be a legitimate bug in the assignment from strided views.

marty1885 commented 7 years ago

@SylvainCorlay I think it's something even deeper than the assignment. After tracing how xarray assigns data from one to another. I come across the function trivial_assigner. After some tinkering I put some debug output into it.

    template <>
    template <class E1, class E2>
    inline void trivial_assigner<false>::run(E1& e1, const E2& e2)
    {
        std::cout << e2.size() << std::endl;
        int s = 0;
        for(auto i : e2)
            s++;
        std::cout << s << std::endl;
        std::copy(e2.storage_cbegin(), e2.storage_cend(), e1.storage_begin());
    }

Then it prints

4
3

So, apparently e2.size() is 4 but the iterator only iterate 3 time? It looks like this is a problem inside the container. This seems deep.

JohanMabille commented 7 years ago

This was a bug in the strided view only, the iterator pointing to the end of the underlying data container was not correctly set for containers having a single element in their leading dimension (2-d containers with a single row or a single column for instance).

Thanks again for reporting!

SylvainCorlay commented 7 years ago

@marty1885 FYI, the issue was merely with the strided view. We have made a release (0.10.11) with that bug fix.

FYI, the issue was due to an optimization that we make in iteration along dimenions of size 1.

burnpanck commented 6 years ago

I see some similar issue with xtensor 0.17.0, xtensor-python 0.20.0 and xsimd 6.1.4 within a larger program: I assign a view(a, all(),range(0,1)) of shape (5,3) to a const pytensor<double,2> &a to a xtensor<double,2> b. The resulting b has the correct shape ((5,1)) but is filled with the value of a(0,0). Could this be related? So far it's still deep in the python extension module, I'll try to factorise to minimal failing case.

wolfv commented 6 years ago

@burnpanck yes, a small reproducer would be very much appreciated.

burnpanck commented 6 years ago

Did some more testing. I believe now to have confirmation that this is indeed a bug, so I created a separate issue.