transpose(E && e) matches too greedily

[ukoethe]

I'm making good progress deriving my own array classes from xview_semantic (thanks for suggesting this approach, @SylvainCorlay!), but encountered a problem that I cannot solve on my own. I declare the array view and transpose function like this:

namespace xvigra
{
    template <index_t N, class T>
    class view_nd
    : public xt::xiterable<view_nd<N, T>>
    , public xt::xview_semantic<view_nd<N, T>>
    {
        ...
    };

    template <index_t N, class T>
    auto transpose(view_nd<N, T> const & array);
}

Unfortunately, I cannot call my transpose function:

view_nd<2, float> v = ...;
auto t = transpose(v);  // calls xt::transpose()

Since my class is derived from xt::xview_semantic, namespace xt participates in name lookup, and xt::transpose(), whose argument is a universal reference, is a better match than xvigra::transpose(). The straightforward idea to add a concept check for xt::is_xexpression to xtensor's function does not work, because view_nd fulfills the concept as well. What can I do to get my function called?

[ukoethe]

I also attempted to use xt::transpose(), but it doesn't work either:

auto t = xt::transpose(v);

results in an exception: "cannot compute transposed layout of dynamic layout". Moreover,

auto t = xt::eval(xt::transpose(v));

fails to compile with error message

/home/ukoethe/src/xtensor/include/xtensor/xstrided_view.hpp:623:40: error: 
'const class xt::detail::expression_adaptor<xvigra::view_nd<3, unsigned char>&>' 
has no member named 'begin'
         return data_xbegin_impl(m_data.begin());

[SylvainCorlay]

What can I do to get my function called?

I guess you can fully qualify your function call as ::xvigra::transpose, but ideally we should get the original transpose to work.

[ukoethe]

1. Full qualification works, but is not a solution I'd like to force on xvigra users -- it's just too easily forgotten.
2. transpose() is just an example for the general problem. Deriving from an xtensor class should not prevent me from having like-named functions in my own namespace to take precedence.
3. I've added metadata to my array classes that are not propagated through xexpressions. In case of transpose and some other functions, I want these metadata to be preserved. Maybe this problem can eventually be solved by an extension of the xexpression framework, but for now, xvigra::transpose() is doing what I want

[SylvainCorlay]

Full qualification works, but is not a solution I'd like to force on xvigra users -- it's just too easily forgotten.

Totally agree. I just meant it as a workaround for you to be able to move forward.

transpose() is just an example for the general problem. Deriving from an xtensor class should not prevent me from having like-named functions in my own namespace to take precedence.
I've added metadata to my array classes that are not propagated through xexpressions. In case of transpose and some other functions, I want these metadata to be preserved. Maybe this problem can eventually be solved by an extension of the xexpression framework, but for now, xvigra::transpose() is doing what I want.

I think that it would be reasonable for us to at least specialize transpose for xexpressions, which does not solve your specific issue, but would allow people to use it in an argument dependent lookup.

On whether we would want like-named functions in your own namespace to take precedence, I need to think about whether it makes sense from an ADL perspective. (It is easy to get it wrong, so we should probably think more about it before being opinionated).

Regarding the "extension of the xexpression framework", this is something that we do for xframe. In fact, xtensor expressions can be extended with a tags system, and reused to make another expression system, but it might be overkill for the vigra usecase.

[ukoethe]

It seems I found a solution: I must also provide

    template <index_t N, class T>
    auto transpose(view_nd<N, T> & array);

Conversion to const made the compiler prefer the universal reference. This is a pretty strange rule... Maybe one can formulate the concept check (or whatever solution) such that I don't have to duplicate all functions for const and non-const references.

[SylvainCorlay]

@ukoethe I spent a bit of time looking at why you encounter in this issue with transpose, which should work normally.

Basically, transpose returns a strided view.

• when the underlying expression is a strided container, it simply reverses the order of the strides.
• when the underlying expression does not have strides, it must be either row-major or column major for us to be able to compute strides of the returned strided view.

The code in question is here.

The test used to check if the underlying expression has strides is not the best one. We basically check the has_raw_data_interface trait.

So one thing you probably want to add in your expression is the raw_data function, like in our containers. When available, xtensor views will be faster, and as a byproduct, the transpose function will work properly.

[ukoethe]

So one thing you probably want to add in your expression is the raw_data function, like in our containers.

I implemented the required API, and xt:transpose(v) now works on my arrays 😄. However, I think it is more natural when the result type of xt::transpose() is the same as (or a close relative of) its argument type. At present, decltype(xt::transpose(v)) has a different API than v for vigra arrays. Can this be fixed?

[SylvainCorlay]

I implemented the required API, and xt:transpose(v) now works on my arrays 😄.

Yeah! Regardless of the discussion on transpose, there are some benefits of having this API in terms of speed.

However, I think it is more natural when the result type of xt::transpose() is the same as (or a close relative of) its argument type. At present, decltype(xt::transpose(v)) has a different API than v for vigra arrays. Can this be fixed?

Well, transpose is a non mutating operation, it returns a view on the passed expression. It only takes ownership of the argument when it is an r value. So in the current implementation, that is not possible with the current implementation.

[ukoethe]

that is not possible with the current implementation

Looking at how the view_type is selected in xt::transpose(), it seems to me that a customization possibility (e.g. via a traits class) wouldn't be too difficult to implement. Any chance that this happens?

[SylvainCorlay]

Maybe a good way to do this is that transpose should be renamed "transposed" and continue returning a view.

You could define transpose as a mutating operation on your containers. A mutating transpose function is another story.

[ukoethe]

You could define transpose as a mutating operation on your containers. A mutating transpose function is another story.

I don't want transpose to be mutating. I just want it to return xvigra::view_nd (which is the same as xstrided_view, just with a different API) when the argument is a vigra array.

[SylvainCorlay]

Gotcha

[ukoethe]

should be renamed "transposed"

This would make sense, but is inconsistent with numpy. So I'd rather vote to keep the established name.
Edit: But, as a matter of fact, I called the function "transposed" for tiny_vector, so I'm undecided.

[wolfv]

@ukoethe what you're suggesting is that xexpression have a type trait such as view_type or transpose_view_type or preferred_view_type to derive the "correct" view type from?
I am undecided if that's a good idea. Seems like a pretty specific change -- on the other hand i can see how people would want to customize the view class.

[JohanMabille]

@wolfv this type traits can be external to the expression, and may not be limited to the transpose function, actually we can propagate it to all the functions returning a strided_view.

[wolfv]

I see, sounds like a good idea.

[ukoethe]

I'm also not sure about the best solution. At the moment, I need four overloads of transpose():

template <class T, index_t N>
decltype(auto) transpose(view_nd<T, N> const & array);

template <class T, index_t N>
decltype(auto) transpose(view_nd<T, N> & array);

template <class T, index_t N, class A>
decltype(auto) transpose(array_nd<T, N, A> const & array);

template <class T, index_t N, class A>
decltype(auto) transpose(array_nd<T, N, A> & array);

If one of the overloads is missing, xtensor::transpose(E && e) may kick in because a universal reference is considered a better match than either a conversion to base reference or a conversion to const reference.

Since xtensor provides many functions taking universal references, it is a general problem to determine exactly when one of those "universal" functions should be called. As a user, I'd like to have some control over the resolution of universal references (e.g. by means of an enable_if), so that the universal function can be switched off when necessary. However, this control should be somewhat fine-grained because I want transpose(view_nd) to call my overload, but sqrt(view_nd) or allclose(view_nd, view_nd) to call the xtensor functions, for example.

[wolfv]

Okay, if I understand @JohanMabille correctly, the idea here would be to do something like

template <class T>
struct get_view_type
{
    template <...>
    using type = xt::dynamic_view<...>;
}

so that you or other library authors can specialize this template, and that's the one we'd be using in functions that return a strided view.

template <>
struct get_view_type<array_nd<...>>
{
   template <...>
   using type = xvigra::view_nd<...>;
}

However, that wouldn't solve any problems with other functions that take universal references ...

[wolfv]

Note that the views would have to be equivalent in the template paramters that they are supplied with (or one can probably drop some params if necessary). We're planning to add a layout template parameter to the view by the way.

[wolfv]

fixed by #765 (now in master). @ukoethe feel free to give us feedback on that (if possible before the next release, probably in one or two weeks we'll release 0.16.0 which will contain a couple of breaking changes).