From a475d4feaa20615e0d6ae3f351baa1bde5f32a23 Mon Sep 17 00:00:00 2001 From: Nick Lester Date: Fri, 6 Nov 2026 15:44:05 -0600 Subject: [PATCH 1/4] perf(utils): check coordinates in a single pass --- include/rabitqlib/utils/input_checks.hpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/include/rabitqlib/utils/input_checks.hpp b/include/rabitqlib/utils/input_checks.hpp index ba883018..306edcba 100644 --- a/include/rabitqlib/utils/input_checks.hpp +++ b/include/rabitqlib/utils/input_checks.hpp @@ -49,15 +49,17 @@ static inline float max_coordinate_magnitude(size_t dim) { static inline void check_coordinates( const float* values, size_t count, size_t dim, const char* what ) { - if (!all_finite(values, count)) { - throw std::invalid_argument(std::string(what) + " must contain only finite values"); - } const float bound = max_coordinate_magnitude(dim); size_t out_of_range = 0; for (size_t i = 0; i < count; ++i) { out_of_range += static_cast(!(std::fabs(values[i]) <= bound)); } if (out_of_range != 0) { + if (!all_finite(values, count)) { + throw std::invalid_argument( + std::string(what) + " must contain only finite values" + ); + } throw std::invalid_argument( std::string(what) + " exceeds the safe float32 coordinate range" ); From 3a3a6bfee244ef7103ed1459b8b1870c194a9ce5 Mon Sep 17 00:00:00 2001 From: Nick Lester Date: Fri, 6 Nov 2026 15:57:09 -0600 Subject: [PATCH 2/4] tests(utils): cover non-finite values mixed with out-of-range values --- .../rabitqlib/utils/input_checks_test.cpp | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/unit/rabitqlib/utils/input_checks_test.cpp b/tests/unit/rabitqlib/utils/input_checks_test.cpp index f0abd877..6639ef78 100644 --- a/tests/unit/rabitqlib/utils/input_checks_test.cpp +++ b/tests/unit/rabitqlib/utils/input_checks_test.cpp @@ -87,6 +87,41 @@ TEST(InputChecksTest, CoordinateBoundMatchesClusteringAndAcceptsItself) { "data must contain only finite values" ); } +TEST(InputChecksTest, NonFiniteWinsOverOversizedFiniteValues) { + constexpr size_t kDim = 128; + const float bound = max_coordinate_magnitude(kDim); + const float oversized = std::nextafter(bound, std::numeric_limits::infinity()); + const auto expect_message = [](const auto& call, const std::string& message) { + try { + call(); + ADD_FAILURE() << "expected: " << message; + } catch (const std::invalid_argument& error) { EXPECT_EQ(error.what(), message); } + }; + for (float non_finite : + {std::numeric_limits::quiet_NaN(), + std::numeric_limits::signaling_NaN(), + std::numeric_limits::infinity(), + -std::numeric_limits::infinity()}) { + // case where oversized finite precedes infinite value: + std::vector values(3, 0.25F); + + values[0] = oversized; + values[2] = non_finite; + + expect_message( + [&] { check_coordinates(values.data(), values.size(), kDim, "data"); }, + "data must contain only finite values" + ); + // case where infinite value precedes oversized value: + values[0] = non_finite; + values[2] = oversized; + + expect_message( + [&] { check_coordinates(values.data(), values.size(), kDim, "data"); }, + "data must contain only finite values" + ); + } +} } // namespace } // namespace rabitqlib From 15f2984af6196b9ddc23b582a061900f8603e12a Mon Sep 17 00:00:00 2001 From: gouyt13clear Date: Sat, 7 Nov 2026 12:48:47 +0800 Subject: [PATCH 3/4] fix(utils): keep infinite values out of range when dim is zero max_coordinate_magnitude(0) is +Inf, so the single-pass range check no longer counted +/-Inf for dim == 0 and check_coordinates accepted them. Cap the bound at FLT_MAX to restore the previous dim == 0 behavior: non-finite values are rejected and every finite value passes. Also share one expect_message helper across the input-check tests and cover the dim == 0 case. --- include/rabitqlib/utils/input_checks.hpp | 5 +- .../rabitqlib/utils/input_checks_test.cpp | 47 ++++++++++++------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/include/rabitqlib/utils/input_checks.hpp b/include/rabitqlib/utils/input_checks.hpp index 306edcba..5b22e94d 100644 --- a/include/rabitqlib/utils/input_checks.hpp +++ b/include/rabitqlib/utils/input_checks.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -49,7 +50,9 @@ static inline float max_coordinate_magnitude(size_t dim) { static inline void check_coordinates( const float* values, size_t count, size_t dim, const char* what ) { - const float bound = max_coordinate_magnitude(dim); + // Capping the bound keeps +/-Inf out of range when dim == 0 makes it infinite. + const float bound = + std::min(max_coordinate_magnitude(dim), std::numeric_limits::max()); size_t out_of_range = 0; for (size_t i = 0; i < count; ++i) { out_of_range += static_cast(!(std::fabs(values[i]) <= bound)); diff --git a/tests/unit/rabitqlib/utils/input_checks_test.cpp b/tests/unit/rabitqlib/utils/input_checks_test.cpp index 6639ef78..e7584809 100644 --- a/tests/unit/rabitqlib/utils/input_checks_test.cpp +++ b/tests/unit/rabitqlib/utils/input_checks_test.cpp @@ -12,6 +12,14 @@ namespace rabitqlib { namespace { +template +void expect_message(const Call& call, const std::string& message) { + try { + call(); + ADD_FAILURE() << "expected: " << message; + } catch (const std::invalid_argument& error) { EXPECT_EQ(error.what(), message); } +} + TEST(InputChecksTest, DetectsEveryNonFiniteValueForFloatAndDouble) { const std::vector finite{ 0.0F, @@ -59,12 +67,6 @@ TEST(InputChecksTest, CoordinateBoundMatchesClusteringAndAcceptsItself) { EXPECT_NO_THROW(check_coordinates(values.data(), values.size(), kDim, "data")); EXPECT_NO_THROW(check_coordinates(values.data(), 0, kDim, "data")); - const auto expect_message = [](const auto& call, const std::string& message) { - try { - call(); - ADD_FAILURE() << "expected: " << message; - } catch (const std::invalid_argument& error) { EXPECT_EQ(error.what(), message); } - }; values[kDim - 1] = std::nextafter(bound, std::numeric_limits::infinity()); expect_message( [&] { check_coordinates(values.data(), values.size(), kDim, "data"); }, @@ -87,41 +89,50 @@ TEST(InputChecksTest, CoordinateBoundMatchesClusteringAndAcceptsItself) { "data must contain only finite values" ); } + TEST(InputChecksTest, NonFiniteWinsOverOversizedFiniteValues) { constexpr size_t kDim = 128; const float bound = max_coordinate_magnitude(kDim); const float oversized = std::nextafter(bound, std::numeric_limits::infinity()); - - const auto expect_message = [](const auto& call, const std::string& message) { - try { - call(); - ADD_FAILURE() << "expected: " << message; - } catch (const std::invalid_argument& error) { EXPECT_EQ(error.what(), message); } - }; for (float non_finite : {std::numeric_limits::quiet_NaN(), std::numeric_limits::signaling_NaN(), std::numeric_limits::infinity(), -std::numeric_limits::infinity()}) { - // case where oversized finite precedes infinite value: + // The oversized finite value precedes the non-finite value. std::vector values(3, 0.25F); - values[0] = oversized; values[2] = non_finite; - expect_message( [&] { check_coordinates(values.data(), values.size(), kDim, "data"); }, "data must contain only finite values" ); - // case where infinite value precedes oversized value: + // The non-finite value precedes the oversized finite value. values[0] = non_finite; values[2] = oversized; - expect_message( [&] { check_coordinates(values.data(), values.size(), kDim, "data"); }, "data must contain only finite values" ); } } + +TEST(InputChecksTest, ZeroDimensionRejectsOnlyNonFiniteValues) { + // dim == 0 makes the range bound infinite; every finite value is accepted. + std::vector values{ + std::numeric_limits::max(), -std::numeric_limits::max(), 0.25F}; + EXPECT_NO_THROW(check_coordinates(values.data(), values.size(), 0, "data")); + for (float non_finite : + {std::numeric_limits::quiet_NaN(), + std::numeric_limits::infinity(), + -std::numeric_limits::infinity()}) { + values[1] = non_finite; + expect_message( + [&] { check_coordinates(values.data(), values.size(), 0, "data"); }, + "data must contain only finite values" + ); + } +} + } // namespace } // namespace rabitqlib From 7b3ae76ee9eeda87df86fbb649f0b0f3fc7489d6 Mon Sep 17 00:00:00 2001 From: gouyt13clear Date: Sat, 7 Nov 2026 13:09:51 +0800 Subject: [PATCH 4/4] fix(utils): avoid dividing by zero in the dim-zero coordinate bound max_coordinate_magnitude(0) divided by zero before the bound was capped, which the float-divide-by-zero sanitizer reports. Return FLT_MAX for dim == 0 directly so no division happens; behavior is unchanged. --- include/rabitqlib/utils/input_checks.hpp | 10 ++++++---- tests/unit/rabitqlib/utils/input_checks_test.cpp | 3 ++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/include/rabitqlib/utils/input_checks.hpp b/include/rabitqlib/utils/input_checks.hpp index 5b22e94d..dd888e9c 100644 --- a/include/rabitqlib/utils/input_checks.hpp +++ b/include/rabitqlib/utils/input_checks.hpp @@ -1,6 +1,5 @@ #pragma once -#include #include #include #include @@ -39,7 +38,12 @@ static inline bool all_finite(const T* values, size_t count) { // Largest coordinate magnitude whose squared distances, their products in graph // pruning, and rotated residuals stay inside float32; shared with clustering. +// dim == 0 has no such limit; FLT_MAX avoids dividing by zero and still leaves +// +/-Inf out of range. static inline float max_coordinate_magnitude(size_t dim) { + if (dim == 0) { + return std::numeric_limits::max(); + } return static_cast(std::sqrt( std::sqrt(static_cast(std::numeric_limits::max())) / (64.0 * static_cast(dim)) @@ -50,9 +54,7 @@ static inline float max_coordinate_magnitude(size_t dim) { static inline void check_coordinates( const float* values, size_t count, size_t dim, const char* what ) { - // Capping the bound keeps +/-Inf out of range when dim == 0 makes it infinite. - const float bound = - std::min(max_coordinate_magnitude(dim), std::numeric_limits::max()); + const float bound = max_coordinate_magnitude(dim); size_t out_of_range = 0; for (size_t i = 0; i < count; ++i) { out_of_range += static_cast(!(std::fabs(values[i]) <= bound)); diff --git a/tests/unit/rabitqlib/utils/input_checks_test.cpp b/tests/unit/rabitqlib/utils/input_checks_test.cpp index e7584809..7758473c 100644 --- a/tests/unit/rabitqlib/utils/input_checks_test.cpp +++ b/tests/unit/rabitqlib/utils/input_checks_test.cpp @@ -118,7 +118,8 @@ TEST(InputChecksTest, NonFiniteWinsOverOversizedFiniteValues) { } TEST(InputChecksTest, ZeroDimensionRejectsOnlyNonFiniteValues) { - // dim == 0 makes the range bound infinite; every finite value is accepted. + // dim == 0 has no range limit; every finite value is accepted. + EXPECT_EQ(max_coordinate_magnitude(0), std::numeric_limits::max()); std::vector values{ std::numeric_limits::max(), -std::numeric_limits::max(), 0.25F}; EXPECT_NO_THROW(check_coordinates(values.data(), values.size(), 0, "data"));