xtensor-stack / xtensor

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

View in function with array as const reference gives errors #39

Closed wolfv closed 7 years ago

wolfv commented 7 years ago

This seems like a strange bug. The following doesn't work for me:

#include "xtensor/xscalar.hpp"
#include "xtensor/xarray.hpp"
#include "xtensor/xio.hpp"

int main() {

    xt::xarray<double> arr1
      {{1.0, 2.0, 3.0, 9},
       {2.0, 5.0, 7.0, 9},
       {2.0, 5.0, 7.0, 9}};

    auto func = [](const auto& arr1) {
        auto view = make_xview(arr1, 1, xt::all());
        for(const auto& el : view) {
            std::cout << el << " ";
        }
    };
    func(arr1);

}

While this one works perfectly fine:

#include "xtensor/xscalar.hpp"
#include "xtensor/xarray.hpp"
#include "xtensor/xio.hpp"

int main() {

    xt::xarray<double> arr1
      {{1.0, 2.0, 3.0, 9},
       {2.0, 5.0, 7.0, 9},
       {2.0, 5.0, 7.0, 9}};

    auto func = [](const auto arr1) {
        auto view = make_xview(arr1, 1, xt::all());
        for(const auto& el : view) {
            std::cout << el << " ";
        }
    };
    func(arr1);

}
wolfv commented 7 years ago

I am unsure if this is really an issue, but it seems like stripping away const in view_type of the view_stepper doesn't affect the const reference. Which leads to compiler errors when using any of the iterators.

I was playing around with implementing the sum functionality when this bit me.

JohanMabille commented 7 years ago

It's strange the second example you give works, it fails to compile for me; anyway that's not a bug. The loop calls the non const version of iterators (since view isn't const) while the underlying array is const. This fix should work (notice the const before the declaration of view):

#include "xtensor/xscalar.hpp"
#include "xtensor/xarray.hpp"
#include "xtensor/xio.hpp"

int main() {

    xt::xarray<double> arr1
      {{1.0, 2.0, 3.0, 9},
       {2.0, 5.0, 7.0, 9},
       {2.0, 5.0, 7.0, 9}};

    auto func = [](const auto& arr1) {
        const auto view = make_xview(arr1, 1, xt::all());
        for(const auto& el : view) {
            std::cout << el << " ";
        }
    };
    func(arr1);
}
wolfv commented 7 years ago

hmm, your fix doesn't compile for me (gcc 6.2.1).

But this worked

view = xt::make_xview(const_cast<xt::xarray<double>&>(arr1), 1, xt::all());

I've used it to implement dot and sum in crude versions over here: https://github.com/wolfv/xtensor/tree/reduce (just in case there is any interest in this work :)

Btw. I am wondering if there is a good way to handle the axis parameter. It seems hard to "dynamically" fill in the values for make_xview.

Or is there a more proper way of doing the xreducer functionality?

Cheers

JohanMabille commented 7 years ago

Ok, I'll look into it; just to be sure, can you confirm you have the fix of const view in your branch (PR #34 )? The const_cast works but it's a dirty workaround, one should be able to use xview on const containers.

We don't have an easier way to handle axis parameters for the moment, we're still debating on how to do it. Any suggestion is welcome :)

wolfv commented 7 years ago

Oh no, I wasn't. Things move fast around here. Sorry!