From e375f2c1b16a3b7692d9230650f63583865d5382 Mon Sep 17 00:00:00 2001 From: Yuuichi Asahi Date: Wed, 2 Oct 2024 20:22:26 +0900 Subject: [PATCH 1/3] fix: is_out_of_range_value_included function --- common/src/KokkosFFT_utils.hpp | 10 ++++++---- common/unit_test/Test_Utils.cpp | 22 +++++++++++++++++++++- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/common/src/KokkosFFT_utils.hpp b/common/src/KokkosFFT_utils.hpp index bb77a1d4..2c1f702d 100644 --- a/common/src/KokkosFFT_utils.hpp +++ b/common/src/KokkosFFT_utils.hpp @@ -75,11 +75,13 @@ bool is_out_of_range_value_included(const ContainerType& values, IntType max) { static_assert(std::is_same_v, "is_out_of_range_value_included: Container value type must " "match IntType"); - bool is_included = false; - for (auto value : values) { - is_included = value >= max; + const auto [vmin, vmax] = std::minmax_element(values.begin(), values.end()); + if constexpr (std::is_signed_v) { + KOKKOSFFT_THROW_IF( + *vmin < 0, + "is_out_of_range_value_included: values must be non-negative"); } - return is_included; + return *vmax >= max; } template < diff --git a/common/unit_test/Test_Utils.cpp b/common/unit_test/Test_Utils.cpp index 609a714e..ea721ee1 100644 --- a/common/unit_test/Test_Utils.cpp +++ b/common/unit_test/Test_Utils.cpp @@ -332,7 +332,7 @@ void test_has_duplicate_values() { template void test_is_out_of_range_value_included() { using IntType = KokkosFFT::Impl::base_container_value_type; - 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(2))); @@ -342,6 +342,26 @@ void test_is_out_of_range_value_included() { v, static_cast(5))); EXPECT_FALSE(KokkosFFT::Impl::is_out_of_range_value_included( v, static_cast(6))); + + EXPECT_TRUE(KokkosFFT::Impl::is_out_of_range_value_included( + v2, static_cast(2))); + EXPECT_TRUE(KokkosFFT::Impl::is_out_of_range_value_included( + v2, static_cast(3))); + EXPECT_FALSE(KokkosFFT::Impl::is_out_of_range_value_included( + v2, static_cast(5))); + EXPECT_FALSE(KokkosFFT::Impl::is_out_of_range_value_included( + v2, static_cast(6))); + + if constexpr (std::is_signed_v) { + // 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(2)); + }, + std::runtime_error); + } } template From 0d675a3dc20f95a31b4a67d3a854db453192e13f Mon Sep 17 00:00:00 2001 From: Yuuichi Asahi Date: Wed, 2 Oct 2024 20:26:34 +0900 Subject: [PATCH 2/3] use std::max_element and std::min_element seperately to avoid unused variable --- common/src/KokkosFFT_utils.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/src/KokkosFFT_utils.hpp b/common/src/KokkosFFT_utils.hpp index 2c1f702d..44f37592 100644 --- a/common/src/KokkosFFT_utils.hpp +++ b/common/src/KokkosFFT_utils.hpp @@ -75,8 +75,9 @@ bool is_out_of_range_value_included(const ContainerType& values, IntType max) { static_assert(std::is_same_v, "is_out_of_range_value_included: Container value type must " "match IntType"); - const auto [vmin, vmax] = std::minmax_element(values.begin(), values.end()); + const auto vmax = std::max_element(values.begin(), values.end()); if constexpr (std::is_signed_v) { + const auto vmin = std::min_element(values.begin(), values.end()); KOKKOSFFT_THROW_IF( *vmin < 0, "is_out_of_range_value_included: values must be non-negative"); From e6e87bd34de11f3ab94989c96e950719171a8f23 Mon Sep 17 00:00:00 2001 From: Yuuichi Asahi Date: Thu, 3 Oct 2024 15:50:39 +0900 Subject: [PATCH 3/3] fix: use any of for is_out_of_range_value_included --- common/src/KokkosFFT_utils.hpp | 8 ++++---- common/unit_test/Test_Utils.cpp | 4 ++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/common/src/KokkosFFT_utils.hpp b/common/src/KokkosFFT_utils.hpp index 44f37592..6e2ca757 100644 --- a/common/src/KokkosFFT_utils.hpp +++ b/common/src/KokkosFFT_utils.hpp @@ -75,14 +75,14 @@ bool is_out_of_range_value_included(const ContainerType& values, IntType max) { static_assert(std::is_same_v, "is_out_of_range_value_included: Container value type must " "match IntType"); - const auto vmax = std::max_element(values.begin(), values.end()); if constexpr (std::is_signed_v) { - const auto vmin = std::min_element(values.begin(), values.end()); KOKKOSFFT_THROW_IF( - *vmin < 0, + 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 *vmax >= max; + 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 index ea721ee1..d104bd9f 100644 --- a/common/unit_test/Test_Utils.cpp +++ b/common/unit_test/Test_Utils.cpp @@ -338,6 +338,8 @@ void test_is_out_of_range_value_included() { v, static_cast(2))); EXPECT_TRUE(KokkosFFT::Impl::is_out_of_range_value_included( v, static_cast(3))); + EXPECT_TRUE(KokkosFFT::Impl::is_out_of_range_value_included( + v, static_cast(4))); EXPECT_FALSE(KokkosFFT::Impl::is_out_of_range_value_included( v, static_cast(5))); EXPECT_FALSE(KokkosFFT::Impl::is_out_of_range_value_included( @@ -347,6 +349,8 @@ void test_is_out_of_range_value_included() { v2, static_cast(2))); EXPECT_TRUE(KokkosFFT::Impl::is_out_of_range_value_included( v2, static_cast(3))); + EXPECT_TRUE(KokkosFFT::Impl::is_out_of_range_value_included( + v2, static_cast(4))); EXPECT_FALSE(KokkosFFT::Impl::is_out_of_range_value_included( v2, static_cast(5))); EXPECT_FALSE(KokkosFFT::Impl::is_out_of_range_value_included(