Fix out of range check (#146)

* fix: is_out_of_range_value_included function * use std::max_element and std::min_element seperately to avoid unused variable * fix: use any of for is_out_of_range_value_included --------- Co-authored-by: Yuuichi Asahi <[email protected]>
kokkos · Oct 3, 2024 · 81ece5b · 81ece5b
1 parent d1d0ada
commit 81ece5b
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 5 deletions.
diff --git a/common/src/KokkosFFT_utils.hpp b/common/src/KokkosFFT_utils.hpp
@@ -75,11 +75,14 @@ bool is_out_of_range_value_included(const ContainerType& values, IntType max) {
   static_assert(std::is_same_v<value_type, IntType>,
                 "is_out_of_range_value_included: Container value type must "
                 "match IntType");
-  bool is_included = false;
-  for (auto value : values) {
-    is_included = value >= max;
+  if constexpr (std::is_signed_v<value_type>) {
+    KOKKOSFFT_THROW_IF(
+        std::any_of(values.begin(), values.end(),
+                    [](value_type value) { return value < 0; }),
+        "is_out_of_range_value_included: values must be non-negative");
   }
-  return is_included;
+  return std::any_of(values.begin(), values.end(),
+                     [max](value_type value) { return value >= max; });
 }
 
 template <

diff --git a/common/unit_test/Test_Utils.cpp b/common/unit_test/Test_Utils.cpp
@@ -332,16 +332,40 @@ void test_has_duplicate_values() {
 template <typename ContainerType>
 void test_is_out_of_range_value_included() {
   using IntType   = KokkosFFT::Impl::base_container_value_type<ContainerType>;
-  ContainerType v = {0, 1, 2, 3, 4};
+  ContainerType v = {0, 1, 2, 3, 4}, v2 = {0, 4, 1};
 
   EXPECT_TRUE(KokkosFFT::Impl::is_out_of_range_value_included(
       v, static_cast<IntType>(2)));
   EXPECT_TRUE(KokkosFFT::Impl::is_out_of_range_value_included(
       v, static_cast<IntType>(3)));
+  EXPECT_TRUE(KokkosFFT::Impl::is_out_of_range_value_included(
+      v, static_cast<IntType>(4)));
   EXPECT_FALSE(KokkosFFT::Impl::is_out_of_range_value_included(
       v, static_cast<IntType>(5)));
   EXPECT_FALSE(KokkosFFT::Impl::is_out_of_range_value_included(
       v, static_cast<IntType>(6)));
+
+  EXPECT_TRUE(KokkosFFT::Impl::is_out_of_range_value_included(
+      v2, static_cast<IntType>(2)));
+  EXPECT_TRUE(KokkosFFT::Impl::is_out_of_range_value_included(
+      v2, static_cast<IntType>(3)));
+  EXPECT_TRUE(KokkosFFT::Impl::is_out_of_range_value_included(
+      v2, static_cast<IntType>(4)));
+  EXPECT_FALSE(KokkosFFT::Impl::is_out_of_range_value_included(
+      v2, static_cast<IntType>(5)));
+  EXPECT_FALSE(KokkosFFT::Impl::is_out_of_range_value_included(
+      v2, static_cast<IntType>(6)));
+
+  if constexpr (std::is_signed_v<IntType>) {
+    // Since non-negative value is included, it should be invalid
+    ContainerType v3 = {0, 1, -1};
+    EXPECT_THROW(
+        {
+          KokkosFFT::Impl::is_out_of_range_value_included(
+              v3, static_cast<IntType>(2));
+        },
+        std::runtime_error);
+  }
 }
 
 template <typename IntType>