Implement review suggestions

Author: sbastrakov

example/complex/CMakeLists.txt

@@ -1,5 +1,5 @@
 #
-# Copyright 2014-2022 Erik Zenker, Benjamin Worpitz, Jan Stephan, Sergei Bastrakov
+# Copyright 2022 Erik Zenker, Benjamin Worpitz, Jan Stephan, Sergei Bastrakov
 #
 # This file exemplifies usage of alpaka.
 #

include/alpaka/math/Complex.hpp

@@ -15,27 +15,36 @@
 #include <cmath>
 #include <complex>
 #include <iostream>
+#include <type_traits>
 
 namespace alpaka
 {
     //! Implementation of a complex number useable on host and device.
     //!
     //! It follows the layout of std::complex and so array-oriented access.
-    //! Inside the class template implements all methods and operators as std::complex<T>.
-    //! Additionally, provides an implicit conversion to and from std::complex<T>.
+    //! The class template implements all methods and operators as std::complex<T>.
+    //! Additionally, it provides an implicit conversion to and from std::complex<T>.
     //! All methods besides operators << and >> are host-device.
-    //! Note that it does not provide non-member functions of std::complex besides the operators.
+    //! It does not provide non-member functions of std::complex besides the operators.
     //! Those are provided the same way as alpaka math functions for real numbers.
     //!
     //! Note that unlike most of alpaka, this is a concrete type template, and not merely a concept.
     //!
-    //! Naming and order of the methods match https://en.cppreference.com/w/cpp/numeric/complex.
+    //! Naming and order of the methods match https://en.cppreference.com/w/cpp/numeric/complex in C++17.
+    //! Implementation chose to not extend it e.g. by adding constexpr to some places that would get it in C++20.
+    //! The motivation is that with internal conversion to std::complex<T> for CPU backends, it would define the common
+    //! interface for genetic code anyways.
+    //! So it is more clear to have alpaka's interface exactly matching when possible, and not "improving".
+    //! \TODO: when switching to C++20, update the interface to match C++20 std::complex<T>.
     //!
     //! @tparam T type of the real and imaginary part: float, double, or long double.
     template<typename T>
     class Complex
     {
     public:
+        // Make sure the input type is floating-point
+        static_assert(std::is_floating_point_v<T>);
+
         //! Type of the real and imaginary parts
         using value_type = T;
 
@@ -307,8 +316,7 @@ namespace alpaka
     template<typename T>
     constexpr ALPAKA_FN_HOST_ACC bool operator!=(Complex<T> const& lhs, Complex<T> const& rhs)
     {
-        return !math::floatEqualExactNoWarning(lhs.real(), rhs.real())
-            || !math::floatEqualExactNoWarning(lhs.imag(), rhs.imag());
+        return !(lhs == rhs);
     }
 
     //! Inequality of a complex and a real number
@@ -381,7 +389,7 @@ namespace alpaka
         return std::acosh(std::complex<T>(x));
     }
 
-    //! Host-side arg falling back to std:: implementation
+    //! Argument
     template<typename T>
     constexpr ALPAKA_FN_HOST T arg(Complex<T> const& x)
     {

include/alpaka/math/Traits.hpp

@@ -166,7 +166,8 @@ namespace alpaka::math
         template<typename T, typename TArgument, typename TSfinae = void>
         struct Arg
         {
-            //!@todo why is this needed here, but not in other functions?
+            // It is unclear why this is needed here and not in other math trait structs. But removing it causes
+            // warnings with calling a __host__ function from a __host__ __device__ function when building for CUDA.
             ALPAKA_NO_HOST_ACC_WARNING
             ALPAKA_FN_HOST_ACC auto operator()(T const& /* ctx */, TArgument const& argument)
             {

test/unit/math/src/Functor.hpp

@@ -61,7 +61,7 @@ namespace alpaka
         template<                                                                                                     \
             typename TAcc,                                                                                            \
             typename... TArgs, /* SFINAE: Enables if called from device. */                                           \
-            typename std::enable_if<!std::is_same<TAcc, std::nullptr_t>::value, int>::type = 0>                       \
+            typename std::enable_if_t<!std::is_same_v<TAcc, std::nullptr_t>, int> = 0>                                \
         ALPAKA_FN_ACC auto execute(TAcc const& acc, TArgs const&... args) const                                       \
         {                                                                                                             \
             return ALPAKA_OP(acc, args...);                                                                           \
@@ -71,7 +71,7 @@ namespace alpaka
         template<                                                                                                     \
             typename TAcc = std::nullptr_t,                                                                           \
             typename TArg1, /* SFINAE: Enables if called from host. */                                                \
-            typename std::enable_if<std::is_same<TAcc, std::nullptr_t>::value, int>::type = 0>                        \
+            typename std::enable_if_t<std::is_same_v<TAcc, std::nullptr_t>, int> = 0>                                 \
         ALPAKA_FN_HOST auto execute(TAcc const& /* acc */, TArg1 const& arg1) const                                   \
         {                                                                                                             \
             return STD_OP(typename StdLibType<TArg1>::type(arg1));                                                    \
@@ -82,7 +82,7 @@ namespace alpaka
             typename TAcc = std::nullptr_t,                                                                           \
             typename TArg1,                                                                                           \
             typename TArg2, /* SFINAE: Enables if called from host. */                                                \
-            typename std::enable_if<std::is_same<TAcc, std::nullptr_t>::value, int>::type = 0>                        \
+            typename std::enable_if_t<std::is_same_v<TAcc, std::nullptr_t>, int> = 0>                                 \
         ALPAKA_FN_HOST auto execute(TAcc const& /* acc */, TArg1 const& arg1, TArg2 const& arg2) const                \
         {                                                                                                             \
             return STD_OP(typename StdLibType<TArg1>::type(arg1), typename StdLibType<TArg2>::type(arg2));            \

test/unit/math/src/mathComplexFloat.cpp

@@ -16,6 +16,7 @@
 
 #include <catch2/catch.hpp>
 
+#include <complex>
 #include <tuple>
 
 using TestAccs = alpaka::test::EnabledAccs<alpaka::DimInt<1u>, std::size_t>;
@@ -34,3 +35,17 @@ TEMPLATE_LIST_TEST_CASE("mathOpsComplexFloat", "[math] [operator]", TestAccFunct
     auto testTemplate = TestTemplate<Acc, Functor>{};
     testTemplate.template operator()<alpaka::Complex<float>>();
 }
+
+#ifdef __has_include
+#    if __has_include(<version>)
+#        include <version>
+#    endif
+#endif
+#ifdef __cpp_lib_is_layout_compatible
+TEMPLATE_LIST_TEST_CASE("mathOpsComplex", "[layout]", TestAccs)
+{
+    // Check both float and double here to avoid duplicating the wrapper code
+    REQUIRE(std::is_layout_compatible_v<alpaka::Complex<float>, std::complex<float>>);
+    REQUIRE(std::is_layout_compatible_v<alpaka::Complex<double>, std::complex<double>>);
+}
+#endif