xtensor-stack / xtensor

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

Iterator issues #2008

Open feribg opened 4 years ago

feribg commented 4 years ago

I'm not sure why but when installing 0.21.4 from conda, it's missing the xaxis_slice_iterator header. Further I can't quite tell the difference between xaxis_slice_iterator and xaxis_iterator.

I'm trying to rewrite this code to avoid the access operators and instead use an iterator (not sure about the performance gain to be had but seems the majority of the time is spent in the () calls):

template <class V1, class V2, class V3>
static inline double gs_tridiag_step(
        V1& x_end,
        const V2& bound,
        const xt::xtensor<double,1>& a,
        const xt::xtensor<double,1>& b,
        const xt::xtensor<double,1>& c,
        const V3& d,
        unsigned long n,
        double w)
{
    double err = 0.0;
    auto init = x_end(0);
    auto next_gs = (d(0)-b(0)*x_end(1))/a(0);
    auto next_bound = std::max((1-w)*init+w*next_gs, bound(0));

    x_end(0) = next_bound;
    err = std::max(err, std::abs(next_bound - init));
    for (auto i = 1; i<n; i++) {
        init = x_end(i);
        next_gs = (d(i)-c(i-1)*next_gs-b(i)*x_end(i+1))/a(i);
        next_bound = std::max((1-w)*init+w*next_gs, bound(i));
        x_end(i) = next_bound;
        err = std::max(err, std::abs(next_bound - init));
    }
    init = x_end(n);
    next_gs = (d(n)-c(n-1)*next_gs)/a(n);
    next_bound = std::max((1-w)*init+w*next_gs, bound(n));
    x_end(n) = next_bound;
    err = std::max(err, std::abs(next_bound - init));
    return err; //inf norm

Any kind of call to a reference of xtensor or a view with ref.begin(), fails with the following:

error: call to implicitly-deleted default constructor of 'xt::xiterator_owner_adaptor<xt::xfunction<xt::math::maximum<void>, xt::xfunction<xt::detail::multiplies, xt::xfunction<xt::math::maximum<void>, xt::xfunction<xt::detail::minus, const xt::xview<xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &, xt::xall<unsigned long>, unsigned long, xt::xnewaxis<unsigned long> > &, xt::xscalar<const double &> >, xt::xscalar<int> >, xt::xfunction<xt::math::exp_fun, xt::xfunction<xt::detail::multiplies, xt::xscalar<const double &>, const xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &> > >, xt::xfunction<xt::detail::multiplies, xt::xfunction<xt::math::exp_fun, xt::xfunction<xt::detail::multiplies, xt::xscalar<const double &>, const xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &> >, xt::xfunction<xt::math::maximum<void>, xt::xfunction<xt::detail::minus, xt::xfunction<xt::detail::multiplies, xt::xfunction<xt::math::exp_fun, xt::xfunction<xt::detail::multiplies, xt::xscalar<double>, const xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &> >, const xt::xview<xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &, xt::xall<unsigned long>, unsigned long, xt::xnewaxis<unsigned long> > &>, xt::xfunction<xt::detail::multiplies, xt::xscalar<const double &>, xt::xfunction<xt::math::exp_fun, xt::xfunction<xt::detail::multiplies, xt::xscalar<double>, const xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &> > > >, xt::xscalar<int> > > >, xt::detail::expression_iterator_getter<xt::xfunction<xt::math::maximum<void>, xt::xfunction<xt::detail::multiplies, xt::xfunction<xt::math::maximum<void>, xt::xfunction<xt::detail::minus, const xt::xview<xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &, xt::xall<unsigned long>, unsigned long, xt::xnewaxis<unsigned long> > &, xt::xscalar<const double &> >, xt::xscalar<int> >, xt::xfunction<xt::math::exp_fun, xt::xfunction<xt::detail::multiplies, xt::xscalar<const double &>, const xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &> > >, xt::xfunction<xt::detail::multiplies, xt::xfunction<xt::math::exp_fun, xt::xfunction<xt::detail::multiplies, xt::xscalar<const double &>, const xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &> >, xt::xfunction<xt::math::maximum<void>, xt::xfunction<xt::detail::minus, xt::xfunction<xt::detail::multiplies, xt::xfunction<xt::math::exp_fun, xt::xfunction<xt::detail::multiplies, xt::xscalar<double>, const xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &> >, const xt::xview<xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &, xt::xall<unsigned long>, unsigned long, xt::xnewaxis<unsigned long> > &>, xt::xfunction<xt::detail::multiplies, xt::xscalar<const double &>, xt::xfunction<xt::math::exp_fun, xt::xfunction<xt::detail::multiplies, xt::xscalar<double>, const xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &> > > >, xt::xscalar<int> > > >, xt::layout_type::row_major> >::const_iterator' (aka 'xiterator<xfunction_stepper<xt::math::maximum<void>, xt::xfunction<xt::detail::multiplies, xt::xfunction<xt::math::maximum<void>, xt::xfunction<xt::detail::minus, const xt::xview<xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &, xt::xall<unsigned long>, unsigned long, xt::xnewaxis<unsigned long> > &, xt::xscalar<const double &> >, xt::xscalar<int> >, xt::xfunction<xt::math::exp_fun, xt::xfunction<xt::detail::multiplies, xt::xscalar<const double &>, const xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &> > >, xt::xfunction<xt::detail::multiplies, xt::xfunction<xt::math::exp_fun, xt::xfunction<xt::detail::multiplies, xt::xscalar<const double &>, const xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &> >, xt::xfunction<xt::math::maximum<void>, xt::xfunction<xt::detail::minus, xt::xfunction<xt::detail::multiplies, xt::xfunction<xt::math::exp_fun, xt::xfunction<xt::detail::multiplies, xt::xscalar<double>, const xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &> >, const xt::xview<xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &, xt::xall<unsigned long>, unsigned long, xt::xnewaxis<unsigned long> > &>, xt::xfunction<xt::detail::multiplies, xt::xscalar<const double &>, xt::xfunction<xt::math::exp_fun, xt::xfunction<xt::detail::multiplies, xt::xscalar<double>, const xt::xtensor_container<xt::uvector<double, std::__1::allocator<double> >, 2, xt::layout_type::row_major, xt::xtensor_expression_tag> &> > > >, xt::xscalar<int> > > >, array<unsigned long, max_array_size<array<unsigned long, 2>, array<unsigned long, 2> >::value> *, (xt::layout_type)1>')
        xiterator_owner_adaptor(self_type&&);
JohanMabille commented 4 years ago

when installing 0.21.4 from conda, it's missing the xaxis_slice_iterator

Good catch! This file is missing from the header list in the CMakeLists.txt.

I can't quite tell the difference between xaxis_slice_iterator and xaxis_iterator.

xaxis_slice_iterator returns 1-D slices along a given axis while xaxis_iterator returns (N-1)-D slices along the given axis. See these examples.

Any kind of call to a reference of xtensor or a view with ref.begin(), fails with the following [...]

Iterators are not expressions, you need to dereference them to get a view:

auto xend1 = x_end(1);
auto next_gs = (d(0)-b(0)*(*xend1)/a(0);
feribg commented 4 years ago

But in the example above x_end is actually a view and not an iterator. The error is triggered on something like x_end.begin()

JohanMabille commented 4 years ago

Ah my bad, I read too fast. Do you have a minimal example to reproduce the error you posted? Iterators are heavily tested and work with xtensor and view objects, so I guess there's something else.

feribg commented 4 years ago

Here's a sample, both fail, I'm more interested in the second one really

xt::xtensor<double, 2> U = xt::zeros<double>({1000L,1000L});
auto step_result_v = xt::view(U, 1, xt::range(1, 999));
auto it = U.begin();
auto end = U.end();
 xt::xtensor<double, 2> U = xt::zeros<double>({1000L,1000L});
 auto step_result_v = xt::view(U, 1, xt::range(1, 999));
 auto it = step_result_v.begin();
 auto end = step_result_v.end();
JohanMabille commented 4 years ago

Can you precise which compiler you use? I could not reproduce the issue with the CI, see #2011

feribg commented 4 years ago

I just ran that unit test and it works fine. I think what happened is my code was working on .21.3 then I couldn't find the header above so I upgraded to 0.21.4 and now the code fails with an obscure iterator template, which doesn't seem to be what I pasted above.

Here's the full compiler error if that helps to narrow it down. It's weird because I don't see any iterators being used in my code so seems like the header includes are causing it. Wondering if it's a library version conflict: https://pastebin.com/dTB9fZgk

Apple clang version 11.0.3 (clang-1103.0.32.29)
Target: x86_64-apple-darwin19.3.0

nlohmann_json version: 3.7.3
benchmark version: 0.0.0
xtensor version: 0.21.4
xsimd version: 7.4.6
xtl version: 0.6.13
blas version: 
lapack version: 
blaze version: 3.7
JohanMabille commented 4 years ago

Can you try with the master branch? If not, can you copy paste the missing header and test again?

If you still have the issue, please post the few lines of code that trigger the error you posted above, because for now I cannot rely it to anything.

feribg commented 4 years ago

I can't easily try master because it relies on conda to install all the dependencies. But when I downgrade to 0.21.3 it builds fine. Here's the full snippet that breaks:

This is on a mac with the clang version mentioned. It's not possible to be a LAPACK related issue right? Because I added Blaze to the build and it conflicted with xtensor-blas so I removed the latter, but I don't see that impacting the xtensor core.

https://gist.github.com/feribg/5a9966960b0a6a949fec7c0c9ee3d77a

JohanMabille commented 4 years ago

I can't easily try master because it relies on conda to install all the dependencies. But when I downgrade to 0.21.3 it builds fine.

One solution is to manullay copy / paste the missing file in your conda environment. The headers of xtensor are located in $CONDA_PREFIX/include/xtensor. Or wait a few days for the next release.

Here's the full snippet that breaks:

I meant "the few lines of code", this is a big long for investigating ;) The idea when we ask for snippets to reproduce is that we can copy paste them and start investigating immediatly without having to fix the code nor having "noise" around what produces the error. Ideally we will add your snippet as a new unit test and run the CI to check for specific compiler issue if we cannot reproduce locally.

This is on a mac with the clang version mentioned. It's not possible to be a LAPACK related issue right? Because I added Blaze to the build and it conflicted with xtensor-blas so I removed the latter, but I don't see that impacting the xtensor core.

I don't think so, especially if you don't include any header from xtensor-blas / lapack in your snippet (thus the importance to narrow down the code to have the few lines required to reproduce).