mdspan icon indicating copy to clipboard operation
mdspan copied to clipboard

Add constructor from mdspan to mdarray

Open gonzalobg opened this issue 2 years ago • 6 comments

It would be really nice to have a conversion from mdspan to an mdarray with suitable extents and layouts.

For one dimensional arrays and spans, layout_left vs layout_right should not make a difference.

It would also be nice to have a conversion from 1D mdpsan and mdarrays with compile-time extents to std::array.

gonzalobg avatar Jul 01 '22 15:07 gonzalobg

Hi Gonzalo! Thanks for reporting this!

The proposal (P1684) actually includes both a constructor from mdspan, and an assignment operator from mdspan. These features are just missing from the reference implementation.

For one dimensional arrays and spans, layout_left vs layout_right should not make a difference.

It doesn't make a difference in the proposal. Assignment from layout_left to layout_right or vice versa already works for mdspan, and should work with mdspan-to-mdarray as well, given the current state of the proposal. This is likely an implementation bug.

It would also be nice to have a conversion from 1D mdspan and mdarrays with compile-time extents to std::array.

This is a new feature not in the proposal. mdspan conversion would look like the following example: https://godbolt.org/z/h6PeTdzPo

namespace stdex = std::experimental;

template<class ElementType, class IndexType, std::size_t N, class Accessor>
requires( N != std::dynamic_extent )
std::array<ElementType, N>
mdspan_to_array(stdex::mdspan<ElementType, stdex::extents<IndexType, N>, stdex::layout_right, Accessor> m)
{
    return [&]<std::size_t ... Indices>(std::integer_sequence<IndexType, Indices...>) {
        return std::array<std::remove_cv_t<ElementType>, N>{ m[Indices]... };
    }( std::make_integer_sequence<IndexType, N>() );
}

except that it might be a conversion operator or the like. This could be added to mdspan as a follow-on proposal after C++23. The analogous mdarray-to-array conversion would look similar.

mhoemmen avatar Jul 01 '22 15:07 mhoemmen

@gonzalobg I've added the proposal feature requests as https://github.com/ORNL/cpp-proposals-pub/issues/278 (already existing) and https://github.com/ORNL/cpp-proposals-pub/issues/280 (new).

The missing mdspan-to-mdarray constructor and assignment operator are implementation bugs.

mhoemmen avatar Jul 01 '22 15:07 mhoemmen

I don't think that mdspan -> std::array makes sense. mdspan is non-owning, std::array isn't. You can't convert std::string_view to std::string or construct a std::array from a std::span either.

crtrott avatar Jul 05 '22 17:07 crtrott

I think this should be a constructor, like the constructors that std::string has from std::string_view, achar*, etc.

However, the C++ language that we have today, combined with the guarantees provided by std::array, prevents us - AFAICT - from adding any constructors to std::array.

So while I'd prefer a constructor, I think that for anything involving std::array, a conversion operator would be more practical.

gonzalobg avatar Jul 05 '22 18:07 gonzalobg

Hm for some reason with the compiler I had the std::string from string_view didn't compile, but I see it has that constructor. However my point about span -> array stands doesn't it? I.e. you can't create an array from a span? If so I still think we first need to make that work before we can do it for mdspan (or at the same time).

crtrott avatar Jul 05 '22 19:07 crtrott

However my point about span -> array stands doesn't it? I.e. you can't create an array from a span?

Yes, that's not supported.

And I agree, I think this should be supported for both std::span and std::mdspan.

gonzalobg avatar Jul 05 '22 19:07 gonzalobg

We added constructor from mspan in the proposal.

crtrott avatar Oct 10 '22 21:10 crtrott

Thank you Christian, happy to hear that!

gonzalobg avatar Oct 10 '22 21:10 gonzalobg