LLNL / Aluminum

High-performance, GPU-aware communication library
https://aluminum.readthedocs.io/en/latest/
Other
84 stars 21 forks source link

Tests use invalid integer types in std::uniform_int_distribution #209

Closed yurivict closed 1 year ago

yurivict commented 1 year ago
-- Build files have been written to: /usr/ports/net/aluminum/work/.build
[ 50% 1/2] /usr/local/libexec/ccache/c++  -I/usr/ports/net/aluminum/work/Aluminum-1.4.0/include -I/usr/ports/net/aluminum/work/.build -I/usr/ports/net/aluminum/work/Aluminum-1.4.0/test -isystem /usr/ports/net/aluminum/work/Aluminum-1.4.0/third_party/cxxopts/include -isystem /usr/local/include -isystem /usr/local/mpi/openmpi/include -O2 -pipe -fstack-protector-strong -fno-strict-aliasing -Wall -Wextra -pedantic -faligned-new -g3 -O2 -pipe -fstack-protector-strong -fno-strict-aliasing   -DNDEBUG -std=gnu++17 -fPIE -fexceptions -pthread -MD -MT test/CMakeFiles/test_ops.dir/test_ops.cpp.o -MF test/CMakeFiles/test_ops.dir/test_ops.cpp.o.d -o test/CMakeFiles/test_ops.dir/test_ops.cpp.o -c /usr/ports/net/aluminum/work/Aluminum-1.4.0/test/test_ops.cpp
FAILED: test/CMakeFiles/test_ops.dir/test_ops.cpp.o 
/usr/local/libexec/ccache/c++  -I/usr/ports/net/aluminum/work/Aluminum-1.4.0/include -I/usr/ports/net/aluminum/work/.build -I/usr/ports/net/aluminum/work/Aluminum-1.4.0/test -isystem /usr/ports/net/aluminum/work/Aluminum-1.4.0/third_party/cxxopts/include -isystem /usr/local/include -isystem /usr/local/mpi/openmpi/include -O2 -pipe -fstack-protector-strong -fno-strict-aliasing -Wall -Wextra -pedantic -faligned-new -g3 -O2 -pipe -fstack-protector-strong -fno-strict-aliasing   -DNDEBUG -std=gnu++17 -fPIE -fexceptions -pthread -MD -MT test/CMakeFiles/test_ops.dir/test_ops.cpp.o -MF test/CMakeFiles/test_ops.dir/test_ops.cpp.o.d -o test/CMakeFiles/test_ops.dir/test_ops.cpp.o -c /usr/ports/net/aluminum/work/Aluminum-1.4.0/test/test_ops.cpp
In file included from /usr/ports/net/aluminum/work/Aluminum-1.4.0/test/test_ops.cpp:28:
In file included from /usr/ports/net/aluminum/work/Aluminum-1.4.0/include/Al.hpp:39:
In file included from /usr/include/c++/v1/vector:312:
In file included from /usr/include/c++/v1/algorithm:1851:
In file included from /usr/include/c++/v1/__algorithm/ranges_sample.h:13:
In file included from /usr/include/c++/v1/__algorithm/sample.h:18:
/usr/include/c++/v1/__random/uniform_int_distribution.h:162:5: error: static assertion failed due to requirement '__libcpp_random_is_valid_inttype<char>::value': IntType must be a supported integer type
    static_assert(__libcpp_random_is_valid_inttype<_IntType>::value, "IntType must be a supported integer type");
    ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/ports/net/aluminum/work/Aluminum-1.4.0/test/test_utils.hpp:138:36: note: in instantiation of template class 'std::uniform_int_distribution<char>' requested here
  std::uniform_int_distribution<T> rng;
                                   ^
/usr/ports/net/aluminum/work/Aluminum-1.4.0/test/test_utils.hpp:149:14: note: in instantiation of function template specialization 'gen_random_val<char, std::linear_congruential_engine<unsigned int, 48271, 0, 2147483647>, true>' requested here
      v[i] = gen_random_val<T>(g);
             ^
/usr/ports/net/aluminum/work/Aluminum-1.4.0/test/test_utils.hpp:207:30: note: in instantiation of function template specialization 'RandVectorGen<char>::gen<std::linear_congruential_engine<unsigned int, 48271, 0, 2147483647>>' requested here
    return RandVectorGen<T>::gen(count, rng_gen);
                             ^
/usr/ports/net/aluminum/work/Aluminum-1.4.0/test/test_ops.cpp:139:35: note: in instantiation of member function 'VectorType<char, Al::MPIBackend>::gen_data' requested here
    input(VectorType<T, Backend>::gen_data(in_size, comm_wrapper.comm().get_stream())),
                                  ^
/usr/include/c++/v1/__memory/allocator.h:165:28: note: in instantiation of member function 'TestData<Al::MPIBackend, char>::TestData' requested here
        ::new ((void*)__p) _Up(_VSTD::forward<_Args>(__args)...);
                           ^
/usr/include/c++/v1/__memory/allocator_traits.h:290:13: note: (skipping 3 contexts in backtrace; use -ftemplate-backtrace-limit=0 to see all)
        __a.construct(__p, _VSTD::forward<_Args>(__args)...);
            ^
/usr/ports/net/aluminum/work/Aluminum-1.4.0/test/test_ops.cpp:356:14: note: in instantiation of function template specialization 'std::vector<TestData<Al::MPIBackend, char>>::emplace_back<unsigned long &, unsigned long &, CommWrapper<Al::MPIBackend> &>' requested here
        data.emplace_back(in_size, out_size, comm_wrappers[i]);
             ^
/usr/ports/net/aluminum/work/Aluminum-1.4.0/test/test_ops.cpp:419:7: note: in instantiation of function template specialization 'run_test<Al::MPIBackend, char, true>' requested here
      run_test<Backend, T>(parsed_opts);
      ^
/usr/ports/net/aluminum/work/Aluminum-1.4.0/test/test_utils.hpp:321:39: note: in instantiation of function template specialization 'test_dispatcher::operator()<Al::MPIBackend, char>' requested here
    {"char", [&]() { functor.template operator()<Backend, char>(parsed_opts); } },
                                      ^
/usr/ports/net/aluminum/work/Aluminum-1.4.0/test/test_utils.hpp:359:20: note: in instantiation of function template specialization 'dispatch_to_backend_type_helper<Al::MPIBackend, test_dispatcher>' requested here
    {"mpi", [&](){ dispatch_to_backend_type_helper<Al::MPIBackend>(parsed_opts, functor); } },
                   ^
/usr/ports/net/aluminum/work/Aluminum-1.4.0/test/test_ops.cpp:488:3: note: in instantiation of function template specialization 'dispatch_to_backend<test_dispatcher>' requested here
  dispatch_to_backend(parsed_opts, test_dispatcher());
  ^
1 error generated.

clang-15 FreeBSD 13.2

ndryden commented 1 year ago

Thanks for reporting this, it is indeed an issue (although not one that has been triggered with any of our compiler tests internally).

std::uniform_int_distribution does indeed require that its IntType be one of short, int, long, long long, unsigned short, unsigned int, unsigned long, or unsigned long long or the behavior is undefined; presumably throwing an error during compilation when given a type not in the list is perfectly compliant with this.

Sadly, this is different from the set of types that std::is_integral defines to be integers (which is what we currently rely on). This seems silly to me (and also to other people, but it's apparently not a defect and fixing it needs to be done in a feature request), but the standard is what the standard is. Based on the list of types we dispatch to, char, schar (signed char), and uchar (unsigned char) are probably broken. (For whatever reason, we don't dispatch to bool. Edit: Probably because none of our backends actually support this, and several underlying libraries don't, either.)

I will fix this soon.

Edit: Also, this issue has presumably been present ever since we put together this testing framework, since this bit of code hasn't changed in three years. Presumably some standard library developer decided to be more strict.

ndryden commented 1 year ago

I believe this is now fixed in the latest v1.4.1 release. Please reopen if not. Thanks!