Adding strong type assertions to AoSoA

[sslattery]

A user tried to use their own class in an AoSoA which violated the design structure. A static_assert was thrown but it wasn't clear why the user's class was invalid. This PR adds stronger checks. We assert types must be both trivial and fundamental allowing us to do a lot of compile-time work. This prevents users from using their own classes directly as AoSoA members although I would argue that the AoSoA and Tuple was intended to replace the user's particle classes with primitive data.

We are still working to identify if there are valid performance-based use cases for allowing users to use their own classes within AoSoA but for now this makes our restrictions more clear and obvious. For example:

struct MyObject { float data[4]; };

void MyObjectTest()
{
    using DataTypes = Cabana::MemberTypes<MyObject,int>;
    using AoSoA_t = Cabana::AoSoA<DataTypes,TEST_MEMSPACE>;
    AoSoA_t aosoa("aosoa");
}

will now fail to compile as:

[1/2] Building CXX object core/unit_test/CMakeFiles/Slice_test_SERIAL.dir/SERIAL/tstSlice_SERIAL.cpp.o
FAILED: core/unit_test/CMakeFiles/Slice_test_SERIAL.dir/SERIAL/tstSlice_SERIAL.cpp.o
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++   -I/Users/uy7/software/Cabana/gtest -Icore/unit_test -I/Users/uy7/software/Cabana/core/unit_test -Icore/unit_test/SERIAL -I/Users/uy7/software/Cabana/core/src -Icore/src -isystem /Users/uy7/install/kokkos/debug/include -isystem /Users/uy7/install/openmpi-4.0.0/include -Wall -Wextra -pedantic -fcolor-diagnostics -DGTEST_HAS_PTHREAD=0 -g -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk -mmacosx-version-min=10.14   -std=c++14 -MD -MT core/unit_test/CMakeFiles/Slice_test_SERIAL.dir/SERIAL/tstSlice_SERIAL.cpp.o -MF core/unit_test/CMakeFiles/Slice_test_SERIAL.dir/SERIAL/tstSlice_SERIAL.cpp.o.d -o core/unit_test/CMakeFiles/Slice_test_SERIAL.dir/SERIAL/tstSlice_SERIAL.cpp.o -c core/unit_test/SERIAL/tstSlice_SERIAL.cpp
In file included from core/unit_test/SERIAL/tstSlice_SERIAL.cpp:2:
In file included from /Users/uy7/software/Cabana/core/unit_test/tstSlice.hpp:12:
In file included from /Users/uy7/software/Cabana/core/src/Cabana_AoSoA.hpp:16:
/Users/uy7/software/Cabana/core/src/Cabana_MemberTypes.hpp:106:5: error: static_assert failed due to requirement 'std::is_arithmetic<Test::MyObject>::value' "Member value types must be arithmetic"
    static_assert( std::is_arithmetic<value_type>::value,
    ^              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/uy7/software/Cabana/core/src/Cabana_MemberTypes.hpp:119:35: note: in instantiation of template class 'Cabana::CheckMemberTypesImpl<1, Test::MyObject, int>' requested here
    static constexpr bool value = CheckMemberTypesImpl<size-1, Types...>::value;
                                  ^
/Users/uy7/software/Cabana/core/src/Cabana_AoSoA.hpp:121:20: note: in instantiation of template class 'Cabana::CheckMemberTypes<Cabana::MemberTypes<Test::MyObject, int> >' requested here
    static_assert( CheckMemberTypes<DataTypes>::value, "AoSoA data type failure" );
                   ^
/Users/uy7/software/Cabana/core/unit_test/tstSlice.hpp:348:13: note: in instantiation of template class 'Cabana::AoSoA<Cabana::MemberTypes<Test::MyObject, int>, Kokkos::HostSpace, 16, Kokkos::MemoryTraits<0> >' requested here
    AoSoA_t aosoa("aosoa");
            ^
1 error generated.
ninja: build stopped: subcommand failed.

Which clearly indicates that the MyObject class is not valid.

[rfbird]

Per our conversation, LGTM.

A few additional notes:

1. If a user wants to store structs, they should just use a Kokkos::View
2. If someone can come up with a use case where storing a struct/class makes sense, we're open to changing this. It's messy for us to mix such differently sizes objects though

[codecov-io]

Codecov Report

Merging #203 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #203   +/-   ##
======================================
  Coverage    47.6%   47.6%           
======================================
  Files          25      25           
  Lines        2692    2692           
======================================
  Hits         1283    1283           
  Misses       1409    1409

Flag	Coverage Δ
#clang	47.6% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c4bf3f...987d2da. Read the comment docs.

[rfbird]

Per our conversation, LGTM.

[weinbe2]

The one possible exception that comes to mind is "fundamental" float or double, which of course is an eternally sore subject as evidenced by every library ever having their own.

Playing devil's advocate on myself:

It also has the trivial workaround of using a float[2]/double[2], at the expense of some loss of code cleanliness.

There could also be an argument against even that for CPU side codes---someone with more CPU vectorization experience should comment, or with Kokkos kernels complex (I think it's there?) may be able to shed some light.

I know that with the correct alignas CUDA will do the appropriate loads and stores, and you can do operator overloading to make the arithmetic look clean. Again I'm not sure how CPU vectorization will do there.

[sslattery]

The one possible exception that comes to mind is "fundamental" float or double, which of course is an eternally sore subject as evidenced by every library ever having their own.

Part of the issue here is how we are computing strides between SoAs in a slice - right now we do it in terms of the number of elements as if the memory was allocated as an array over the type which the slice exists. So non-arithmetic types break this. We can still handle arbitrary user types if they are trivial but that would require not using Kokkos::View in the Slice implementation, doing our own casting/pointer math, and losing all the nice stuff we get from the Kokkos::View in the implementation (e.g. memory traits, debug bounds checking, etc.). If we see a compelling performance-related use case that can't be implemented any other way then we would be open to changing.

[weinbe2]

@sslattery sorry, I had a big typo there, I look like an idiot right now.

I meant a "fundamental" complex float or double.

[weinbe2]

That said, that doesn't fundamentally change the point you're making.

[dalg24]

s/valied/valid/

Also from that description I cannot tell how this differs from is_member_type

[dalg24]

Only built-in types will have a specialization of is_arithmetic with a value that is true and all are trivial as far as I know.