While looking to add a deduction guide to Kokkos::Array, I had to add unit tests, which uncovered some issues.
Kokkos::Array<> is not an array but has two tag typescontiguous and strided which are used in other specialisations of Kokkos::Array.
Kokkos::Array<T, N> is an owning array similar to std::array without iterator support.
Kokkos::Array<T, KOKKOS_INVALID_INDEX, Kokkos::Array<>::contiguous> is a non-owning array (a span-like type) around contiguous storage. The assignment operators have funky semantics (a=b only copies all of b if a.size() == b.size()).
Kokkos::Array<T, KOKKOS_INVALID_INDEX, Kokkos::Array<>::strided> is a non-owning array (a span-like type) around strided storage. The assignment operators have funky semantics (a=b only copies all of b if a.size() == b.size()). The assignment operators are also buggy making it obvious that they have never been used.
Q: Should these four things be conflated into the same public type? The only place that contiguous and strided Kokkos::Array is used is in core/src/impl/Kokkos_ViewArray.hpp for defining ViewMapping<Traits, Kokkos::Array<>>::reference_type.
My vote would be for Kokkos::Array to only support owning semantics, and to deprecate/remove the third (Proxy) template parameter. The other semantics can be moved to their own non-templated (private?) types.
Q: I can fix the bugs in the funky strided assignment operators. Should I, or should I just remove this unused functionality?
My vote is to remove the assignment operators from contiguous and strided Kokkos:Array.
Another issue:
The subscripting operators are templated on integral and enum types. I assume (but don't know) this was done to avoid compiler warnings, such as signed/unsigned mismatches. However, while it deliberately supports enums, it doesn't support scoped enums. Also, unlike C array subscripting operators, it doesn't work on things convertible to size_t.
Q: Should scoped enums be supported for subscripting? The reason to do so is that it encourages people to use them over unscoped enums. (If we intend not to support them, I'll give it a better error message.).
My vote is to support scoped enums.
Q: Should type convertible to size_t be supported for subscripting? I don't see why not, and it meets user expectations (especially for new users).
My vote is to support types convertible to size_t.
The plan (breaking up my current PR into these):
Add is_scoped_enum trait with compile time tests.
Add Kokkos::Array unit tests and fix the bugs with contiguous and stridedArray<...>::empty().
Address assignment operators for contiguous and strided.
Add deduction guide and to_Array to match standard C++.
While looking to add a deduction guide to
Kokkos::Array
, I had to add unit tests, which uncovered some issues.Kokkos::Array<>
is not an array but has two tag typescontiguous
andstrided
which are used in other specialisations ofKokkos::Array
.Kokkos::Array<T, N>
is an owning array similar tostd::array
without iterator support.Kokkos::Array<T, KOKKOS_INVALID_INDEX, Kokkos::Array<>::contiguous>
is a non-owning array (a span-like type) around contiguous storage. The assignment operators have funky semantics (a=b
only copies all ofb
ifa.size() == b.size()
).Kokkos::Array<T, KOKKOS_INVALID_INDEX, Kokkos::Array<>::strided>
is a non-owning array (a span-like type) around strided storage. The assignment operators have funky semantics (a=b
only copies all ofb
ifa.size() == b.size()
). The assignment operators are also buggy making it obvious that they have never been used.Q: Should these four things be conflated into the same public type? The only place that contiguous and strided
Kokkos::Array
is used is in core/src/impl/Kokkos_ViewArray.hpp for definingViewMapping<Traits, Kokkos::Array<>>::reference_type
.My vote would be for
Kokkos::Array
to only support owning semantics, and to deprecate/remove the third (Proxy
) template parameter. The other semantics can be moved to their own non-templated (private?) types.Q: I can fix the bugs in the funky strided assignment operators. Should I, or should I just remove this unused functionality?
My vote is to remove the assignment operators from contiguous and strided
Kokkos:Array
.Another issue:
The subscripting operators are templated on integral and enum types. I assume (but don't know) this was done to avoid compiler warnings, such as signed/unsigned mismatches. However, while it deliberately supports enums, it doesn't support scoped enums. Also, unlike C array subscripting operators, it doesn't work on things convertible to
size_t
.Q: Should scoped enums be supported for subscripting? The reason to do so is that it encourages people to use them over unscoped enums. (If we intend not to support them, I'll give it a better error message.).
My vote is to support scoped enums.
Q: Should type convertible to
size_t
be supported for subscripting? I don't see why not, and it meets user expectations (especially for new users).My vote is to support types convertible to
size_t
.The plan (breaking up my current PR into these):
is_scoped_enum
trait with compile time tests.Kokkos::Array
unit tests and fix the bugs withcontiguous
andstrided
Array<...>::empty()
.contiguous
andstrided
.to_Array
to match standard C++.size_t
in subscripting.Kokkos::Array
.