From 61951784c36d290762b4c7e192d7b9402e4109f9 Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Tue, 1 Jul 2025 23:46:44 -0400 Subject: [PATCH 01/15] Add example tests mentioned in issue Temporary for now --- cpp/src/arrow/scalar_test.cc | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 422f688957ae..985498817df2 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -39,6 +39,7 @@ #include "arrow/testing/random.h" #include "arrow/testing/util.h" #include "arrow/type_traits.h" +#include "arrow/util/float16.h" namespace arrow { @@ -1181,7 +1182,34 @@ TEST(TestDayTimeIntervalScalars, Basics) { ASSERT_TRUE(first->Equals(ts_val2)); } -// TODO test HalfFloatScalar +TEST(TestHalfFloatScalar, Basics) { + auto f1 = util::Float16::FromDouble(+0.0); + auto f2 = util::Float16::FromDouble(-0.0); + ASSERT_TRUE(f1.is_zero()); + ASSERT_TRUE(f2.is_zero()); + ASSERT_TRUE(f2.signbit()); + HalfFloatScalar scalar_1(f1.bits()); + HalfFloatScalar scalar_2(f2.bits()); + + f1 = util::Float16::FromBits(scalar_1.value); + f2 = util::Float16::FromBits(scalar_2.value); + ASSERT_TRUE(f1.is_zero()); + ASSERT_TRUE(f2.is_zero()); + ASSERT_TRUE(f2.signbit()); + EXPECT_FALSE( + scalar_1.Equals(scalar_2, EqualOptions::Defaults().signed_zeros_equal(false))); + EXPECT_TRUE( + scalar_1.Equals(scalar_2, EqualOptions::Defaults().signed_zeros_equal(true))); +} + +TEST(TestHalfFloatArray, Basics) { + auto half_float_array = ArrayFromJSON(float16(), R"([16.0,NaN])"); + auto half_float_array_1 = ArrayFromJSON(float16(), R"([16.0,NaN])"); + EXPECT_TRUE(half_float_array->Equals(half_float_array_1, + EqualOptions::Defaults().nans_equal(true))); + EXPECT_FALSE(half_float_array->Equals(half_float_array, + EqualOptions::Defaults().nans_equal(false))); +} TYPED_TEST(TestNumericScalar, Cast) { auto type = TypeTraits::type_singleton(); From 93b8a0a658caf06f44376587432f8c63791c73ac Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Tue, 1 Jul 2025 23:57:33 -0400 Subject: [PATCH 02/15] Fix Float16 zero/NaN comparisons --- cpp/src/arrow/compare.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/compare.cc b/cpp/src/arrow/compare.cc index 76fd47119e57..d37325fa1ad2 100644 --- a/cpp/src/arrow/compare.cc +++ b/cpp/src/arrow/compare.cc @@ -110,7 +110,7 @@ struct FloatingEquality { bool operator()(uint16_t x, uint16_t y) const { Float16 f_x = Float16::FromBits(x); Float16 f_y = Float16::FromBits(y); - if (x == y) { + if (f_x == f_y) { return Flags::signed_zeros_equal || (f_x.signbit() == f_y.signbit()); } if (Flags::nans_equal && f_x.is_nan() && f_y.is_nan()) { @@ -171,7 +171,8 @@ void VisitFloatingEquality(const EqualOptions& options, bool floating_approximat } inline bool IdentityImpliesEqualityNansNotEqual(const DataType& type) { - if (type.id() == Type::FLOAT || type.id() == Type::DOUBLE) { + if (type.id() == Type::FLOAT || type.id() == Type::DOUBLE || + type.id() == Type::HALF_FLOAT) { return false; } for (const auto& child : type.fields()) { From 187877b5f3a9a28abd9603435a754493b7d0b4bb Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Thu, 3 Jul 2025 20:11:16 -0400 Subject: [PATCH 03/15] Exclude Float16 NaNs from being randomly generated --- cpp/src/arrow/testing/random.cc | 53 +++++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/testing/random.cc b/cpp/src/arrow/testing/random.cc index 2d6ba44d7e30..b6ee93e32ea3 100644 --- a/cpp/src/arrow/testing/random.cc +++ b/cpp/src/arrow/testing/random.cc @@ -43,6 +43,7 @@ #include "arrow/util/bitmap_reader.h" #include "arrow/util/checked_cast.h" #include "arrow/util/decimal.h" +#include "arrow/util/float16.h" #include "arrow/util/key_value_metadata.h" #include "arrow/util/logging_internal.h" #include "arrow/util/pcg_random.h" @@ -54,12 +55,13 @@ namespace arrow { using internal::checked_cast; using internal::checked_pointer_cast; using internal::ToChars; +using util::Float16; namespace random { namespace { -template +template struct GenerateOptions { GenerateOptions(SeedType seed, ValueType min, ValueType max, double probability, double nan_probability = 0.0) @@ -101,8 +103,19 @@ struct GenerateOptions { pcg32_fast rng(seed_++); DistributionType dist(min_, max_); - // A static cast is required due to the int16 -> int8 handling. - std::generate(data, data + n, [&] { return static_cast(dist(rng)); }); + if constexpr (std::is_same_v) { + // Special handling is required to prevent generating Float16 NaNs + std::generate(data, data + n, [&] { + Float16 f; + do { + f = Float16::FromBits(static_cast(dist(rng))); + } while (f.is_nan()); + return f.bits(); + }); + } else { + // A static cast is required due to the int16 -> int8 handling. + std::generate(data, data + n, [&] { return static_cast(dist(rng)); }); + } } void GenerateBitmap(uint8_t* buffer, size_t n, int64_t* null_count) { @@ -228,8 +241,6 @@ PRIMITIVE_RAND_INTEGER_IMPL(UInt32, uint32_t, UInt32Type) PRIMITIVE_RAND_INTEGER_IMPL(Int32, int32_t, Int32Type) PRIMITIVE_RAND_INTEGER_IMPL(UInt64, uint64_t, UInt64Type) PRIMITIVE_RAND_INTEGER_IMPL(Int64, int64_t, Int64Type) -// Generate 16bit values for half-float -PRIMITIVE_RAND_INTEGER_IMPL(Float16, int16_t, HalfFloatType) std::shared_ptr RandomArrayGenerator::Date64(int64_t size, int64_t min, int64_t max, double null_probability, @@ -241,6 +252,20 @@ std::shared_ptr RandomArrayGenerator::Date64(int64_t size, int64_t min, memory_pool); } +std::shared_ptr RandomArrayGenerator::Float16(int64_t size, int16_t min, + int16_t max, double null_probability, + int64_t alignment, + MemoryPool* memory_pool) { + using OptionType = + GenerateOptions, HalfFloatType>; + // FIXME: Not sure why the input min/max are signed when Float16's ctype is uint16_t + uint16_t umin = static_cast(min); + uint16_t umax = static_cast(max); + OptionType options(seed(), umin, umax, null_probability, /*nan_probability=*/0); + return GenerateNumericArray(size, options, alignment, + memory_pool); +} + std::shared_ptr RandomArrayGenerator::Float32(int64_t size, float min, float max, double null_probability, double nan_probability, @@ -1089,10 +1114,26 @@ std::shared_ptr RandomArrayGenerator::ArrayOf(const Field& field, int64_t GENERATE_INTEGRAL_CASE(Int32Type); GENERATE_INTEGRAL_CASE(UInt64Type); GENERATE_INTEGRAL_CASE(Int64Type); - GENERATE_INTEGRAL_CASE_VIEW(Int16Type, HalfFloatType); GENERATE_FLOATING_CASE(FloatType, Float32); GENERATE_FLOATING_CASE(DoubleType, Float64); + case Type::type::HALF_FLOAT: { + using CType = HalfFloatType::c_type; + const CType min_value = + GetMetadata(field.metadata().get(), "min", + std::numeric_limits<::arrow::util::Float16>::min().bits()); + const CType max_value = + GetMetadata(field.metadata().get(), "max", + std::numeric_limits<::arrow::util::Float16>::max().bits()); + const double nan_probability = + GetMetadata(field.metadata().get(), "nan_probability", 0); + VALIDATE_MIN_MAX(Float16::FromBits(min_value), Float16::FromBits(max_value)); + VALIDATE_RANGE(nan_probability, 0.0, 1.0); + // TODO: New interface to allow passing `nan_probability` + return Float16(length, min_value, max_value, null_probability, alignment, + memory_pool); + } + case Type::type::STRING: case Type::type::BINARY: { const auto min_length = From 3ee96494b1d2408acee4c6304a84ccb3eb2973ce Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Fri, 4 Jul 2025 20:40:36 -0400 Subject: [PATCH 04/15] Update scalar/array tests --- cpp/src/arrow/array/array_test.cc | 76 +++++++++++++++++++++++-------- cpp/src/arrow/scalar_test.cc | 68 +++++++++++++-------------- 2 files changed, 87 insertions(+), 57 deletions(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index 85885ded1451..7f69f45f7f52 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -2120,16 +2120,36 @@ void CheckSliceApproxEquals() { ASSERT_TRUE(slice1->ApproxEquals(slice2)); } +template +auto GetFloat(double d) { + if constexpr (std::is_same_v) { + const auto h = Float16::FromDouble(d); + // Double check that nan/inf/sign are preserved + if (std::isnan(d)) { + EXPECT_TRUE(h.is_nan()); + } + if (std::isinf(d)) { + EXPECT_TRUE(h.is_infinity()); + } + if (std::signbit(d)) { + EXPECT_TRUE(h.signbit()); + } + return h.bits(); + } else { + return static_cast(d); + } +} + template void CheckFloatingNanEquality() { std::shared_ptr a, b; std::shared_ptr type = TypeTraits::type_singleton(); - const auto nan_value = static_cast(NAN); + const auto nan_value = GetFloat(NAN); // NaN in a null entry - ArrayFromVector(type, {true, false}, {0.5, nan_value}, &a); - ArrayFromVector(type, {true, false}, {0.5, nan_value}, &b); + ArrayFromVector(type, {true, false}, {GetFloat(0.5), nan_value}, &a); + ArrayFromVector(type, {true, false}, {GetFloat(0.5), nan_value}, &b); ASSERT_TRUE(a->Equals(b)); ASSERT_TRUE(b->Equals(a)); ASSERT_TRUE(a->ApproxEquals(b)); @@ -2140,8 +2160,8 @@ void CheckFloatingNanEquality() { ASSERT_TRUE(b->RangeEquals(a, 1, 2, 1)); // NaN in a valid entry - ArrayFromVector(type, {false, true}, {0.5, nan_value}, &a); - ArrayFromVector(type, {false, true}, {0.5, nan_value}, &b); + ArrayFromVector(type, {false, true}, {GetFloat(0.5), nan_value}, &a); + ArrayFromVector(type, {false, true}, {GetFloat(0.5), nan_value}, &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_TRUE(a->Equals(b, EqualOptions().nans_equal(true))); @@ -2160,8 +2180,9 @@ void CheckFloatingNanEquality() { ASSERT_TRUE(b->RangeEquals(a, 0, 1, 0)); // NaN != non-NaN - ArrayFromVector(type, {false, true}, {0.5, nan_value}, &a); - ArrayFromVector(type, {false, true}, {0.5, 0.0}, &b); + ArrayFromVector(type, {false, true}, {GetFloat(0.5), nan_value}, &a); + ArrayFromVector(type, {false, true}, {GetFloat(0.5), GetFloat(0.0)}, + &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_FALSE(a->Equals(b, EqualOptions().nans_equal(true))); @@ -2185,12 +2206,14 @@ void CheckFloatingInfinityEquality() { std::shared_ptr a, b; std::shared_ptr type = TypeTraits::type_singleton(); - const auto infinity = std::numeric_limits::infinity(); + const auto infinity = GetFloat(std::numeric_limits::infinity()); for (auto nans_equal : {false, true}) { // Infinity in a null entry - ArrayFromVector(type, {true, false}, {0.5, infinity}, &a); - ArrayFromVector(type, {true, false}, {0.5, -infinity}, &b); + ArrayFromVector(type, {true, false}, + {GetFloat(0.5), GetFloat(infinity)}, &a); + ArrayFromVector(type, {true, false}, + {GetFloat(0.5), GetFloat(-infinity)}, &b); ASSERT_TRUE(a->Equals(b)); ASSERT_TRUE(b->Equals(a)); ASSERT_TRUE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); @@ -2201,8 +2224,10 @@ void CheckFloatingInfinityEquality() { ASSERT_TRUE(b->RangeEquals(a, 1, 2, 1)); // Infinity in a valid entry - ArrayFromVector(type, {false, true}, {0.5, infinity}, &a); - ArrayFromVector(type, {false, true}, {0.5, infinity}, &b); + ArrayFromVector(type, {false, true}, + {GetFloat(0.5), GetFloat(infinity)}, &a); + ArrayFromVector(type, {false, true}, + {GetFloat(0.5), GetFloat(infinity)}, &b); ASSERT_TRUE(a->Equals(b)); ASSERT_TRUE(b->Equals(a)); ASSERT_TRUE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); @@ -2219,8 +2244,10 @@ void CheckFloatingInfinityEquality() { ASSERT_TRUE(b->RangeEquals(a, 0, 1, 0)); // Infinity != non-infinity - ArrayFromVector(type, {false, true}, {0.5, -infinity}, &a); - ArrayFromVector(type, {false, true}, {0.5, 0.0}, &b); + ArrayFromVector(type, {false, true}, + {GetFloat(0.5), GetFloat(-infinity)}, &a); + ArrayFromVector(type, {false, true}, {GetFloat(0.5), GetFloat(0.0)}, + &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_FALSE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); @@ -2228,8 +2255,10 @@ void CheckFloatingInfinityEquality() { ASSERT_FALSE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); ASSERT_FALSE(b->ApproxEquals(a, EqualOptions().atol(1e-5).nans_equal(nans_equal))); // Infinity != Negative infinity - ArrayFromVector(type, {true, true}, {0.5, -infinity}, &a); - ArrayFromVector(type, {true, true}, {0.5, infinity}, &b); + ArrayFromVector(type, {true, true}, + {GetFloat(0.5), GetFloat(-infinity)}, &a); + ArrayFromVector(type, {true, true}, + {GetFloat(0.5), GetFloat(infinity)}, &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_FALSE(a->ApproxEquals(b)); @@ -2252,8 +2281,10 @@ void CheckFloatingZeroEquality() { std::shared_ptr a, b; std::shared_ptr type = TypeTraits::type_singleton(); - ArrayFromVector(type, {true, false}, {0.0, 1.0}, &a); - ArrayFromVector(type, {true, false}, {0.0, 1.0}, &b); + ArrayFromVector(type, {true, false}, {GetFloat(0.0), GetFloat(1.0)}, + &a); + ArrayFromVector(type, {true, false}, {GetFloat(0.0), GetFloat(1.0)}, + &b); ASSERT_TRUE(a->Equals(b)); ASSERT_TRUE(b->Equals(a)); for (auto nans_equal : {false, true}) { @@ -2269,8 +2300,10 @@ void CheckFloatingZeroEquality() { } } - ArrayFromVector(type, {true, false}, {0.0, 1.0}, &a); - ArrayFromVector(type, {true, false}, {-0.0, 1.0}, &b); + ArrayFromVector(type, {true, false}, {GetFloat(0.0), GetFloat(1.0)}, + &a); + ArrayFromVector(type, {true, false}, {GetFloat(-0.0), GetFloat(1.0)}, + &b); for (auto nans_equal : {false, true}) { auto opts = EqualOptions().nans_equal(nans_equal); ASSERT_TRUE(a->Equals(b, opts)); @@ -2306,16 +2339,19 @@ TEST(TestPrimitiveAdHoc, FloatingSliceApproxEquals) { TEST(TestPrimitiveAdHoc, FloatingNanEquality) { CheckFloatingNanEquality(); CheckFloatingNanEquality(); + CheckFloatingNanEquality(); } TEST(TestPrimitiveAdHoc, FloatingInfinityEquality) { CheckFloatingInfinityEquality(); CheckFloatingInfinityEquality(); + CheckFloatingInfinityEquality(); } TEST(TestPrimitiveAdHoc, FloatingZeroEquality) { CheckFloatingZeroEquality(); CheckFloatingZeroEquality(); + CheckFloatingZeroEquality(); } // ---------------------------------------------------------------------- diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 985498817df2..e289e455640c 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -47,6 +47,7 @@ using compute::Cast; using compute::CastOptions; using internal::checked_cast; using internal::checked_pointer_cast; +using util::Float16; std::shared_ptr CheckMakeNullScalar(const std::shared_ptr& type) { const auto scalar = MakeNullScalar(type); @@ -298,6 +299,26 @@ TYPED_TEST(TestNumericScalar, MakeScalar) { AssertParseScalar(type, "3", ScalarType(3)); } +template +auto GetFloat(double d) { + if constexpr (std::is_same_v) { + const auto h = Float16::FromDouble(d); + // Double check that nan/inf/sign are preserved + if (std::isnan(d)) { + EXPECT_TRUE(h.is_nan()); + } + if (std::isinf(d)) { + EXPECT_TRUE(h.is_infinity()); + } + if (std::signbit(d)) { + EXPECT_TRUE(h.signbit()); + } + return h.bits(); + } else { + return static_cast(d); + } +} + template class TestRealScalar : public ::testing::Test { public: @@ -307,21 +328,21 @@ class TestRealScalar : public ::testing::Test { void SetUp() { type_ = TypeTraits::type_singleton(); - scalar_val_ = std::make_shared(static_cast(1)); + scalar_val_ = std::make_shared(GetFloat(1)); ASSERT_TRUE(scalar_val_->is_valid); - scalar_other_ = std::make_shared(static_cast(1.1)); + scalar_other_ = std::make_shared(GetFloat(1.1)); ASSERT_TRUE(scalar_other_->is_valid); - scalar_zero_ = std::make_shared(static_cast(0.0)); - scalar_other_zero_ = std::make_shared(static_cast(0.0)); - scalar_neg_zero_ = std::make_shared(static_cast(-0.0)); + scalar_zero_ = std::make_shared(GetFloat(0.0)); + scalar_other_zero_ = std::make_shared(GetFloat(0.0)); + scalar_neg_zero_ = std::make_shared(GetFloat(-0.0)); - const CType nan_value = std::numeric_limits::quiet_NaN(); + const CType nan_value = GetFloat(std::numeric_limits::quiet_NaN()); scalar_nan_ = std::make_shared(nan_value); ASSERT_TRUE(scalar_nan_->is_valid); - const CType other_nan_value = std::numeric_limits::quiet_NaN(); + const CType other_nan_value = GetFloat(std::numeric_limits::quiet_NaN()); scalar_other_nan_ = std::make_shared(other_nan_value); ASSERT_TRUE(scalar_other_nan_->is_valid); } @@ -523,7 +544,9 @@ class TestRealScalar : public ::testing::Test { scalar_zero_, scalar_other_zero_, scalar_neg_zero_; }; -TYPED_TEST_SUITE(TestRealScalar, RealArrowTypes); +using RealArrowTypesPlusHalfFloat = + ::testing::Types; +TYPED_TEST_SUITE(TestRealScalar, RealArrowTypesPlusHalfFloat); TYPED_TEST(TestRealScalar, NanEquals) { this->TestNanEquals(); } @@ -1182,35 +1205,6 @@ TEST(TestDayTimeIntervalScalars, Basics) { ASSERT_TRUE(first->Equals(ts_val2)); } -TEST(TestHalfFloatScalar, Basics) { - auto f1 = util::Float16::FromDouble(+0.0); - auto f2 = util::Float16::FromDouble(-0.0); - ASSERT_TRUE(f1.is_zero()); - ASSERT_TRUE(f2.is_zero()); - ASSERT_TRUE(f2.signbit()); - HalfFloatScalar scalar_1(f1.bits()); - HalfFloatScalar scalar_2(f2.bits()); - - f1 = util::Float16::FromBits(scalar_1.value); - f2 = util::Float16::FromBits(scalar_2.value); - ASSERT_TRUE(f1.is_zero()); - ASSERT_TRUE(f2.is_zero()); - ASSERT_TRUE(f2.signbit()); - EXPECT_FALSE( - scalar_1.Equals(scalar_2, EqualOptions::Defaults().signed_zeros_equal(false))); - EXPECT_TRUE( - scalar_1.Equals(scalar_2, EqualOptions::Defaults().signed_zeros_equal(true))); -} - -TEST(TestHalfFloatArray, Basics) { - auto half_float_array = ArrayFromJSON(float16(), R"([16.0,NaN])"); - auto half_float_array_1 = ArrayFromJSON(float16(), R"([16.0,NaN])"); - EXPECT_TRUE(half_float_array->Equals(half_float_array_1, - EqualOptions::Defaults().nans_equal(true))); - EXPECT_FALSE(half_float_array->Equals(half_float_array, - EqualOptions::Defaults().nans_equal(false))); -} - TYPED_TEST(TestNumericScalar, Cast) { auto type = TypeTraits::type_singleton(); From 42cc8498dca0a79b2159ee0705b4bdd3b008ac2f Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Tue, 8 Jul 2025 15:19:28 -0400 Subject: [PATCH 05/15] Move/revise GetFloat test helper --- cpp/src/arrow/array/array_test.cc | 72 +++++++++++------------------- cpp/src/arrow/scalar_test.cc | 35 ++++----------- cpp/src/arrow/testing/gtest_util.h | 19 ++++++++ 3 files changed, 53 insertions(+), 73 deletions(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index 7f69f45f7f52..8c315faeba49 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -2120,36 +2120,16 @@ void CheckSliceApproxEquals() { ASSERT_TRUE(slice1->ApproxEquals(slice2)); } -template -auto GetFloat(double d) { - if constexpr (std::is_same_v) { - const auto h = Float16::FromDouble(d); - // Double check that nan/inf/sign are preserved - if (std::isnan(d)) { - EXPECT_TRUE(h.is_nan()); - } - if (std::isinf(d)) { - EXPECT_TRUE(h.is_infinity()); - } - if (std::signbit(d)) { - EXPECT_TRUE(h.signbit()); - } - return h.bits(); - } else { - return static_cast(d); - } -} - template void CheckFloatingNanEquality() { std::shared_ptr a, b; std::shared_ptr type = TypeTraits::type_singleton(); - const auto nan_value = GetFloat(NAN); + const auto nan_value = RealToCType(NAN); // NaN in a null entry - ArrayFromVector(type, {true, false}, {GetFloat(0.5), nan_value}, &a); - ArrayFromVector(type, {true, false}, {GetFloat(0.5), nan_value}, &b); + ArrayFromVector(type, {true, false}, {RealToCType(0.5), nan_value}, &a); + ArrayFromVector(type, {true, false}, {RealToCType(0.5), nan_value}, &b); ASSERT_TRUE(a->Equals(b)); ASSERT_TRUE(b->Equals(a)); ASSERT_TRUE(a->ApproxEquals(b)); @@ -2160,8 +2140,8 @@ void CheckFloatingNanEquality() { ASSERT_TRUE(b->RangeEquals(a, 1, 2, 1)); // NaN in a valid entry - ArrayFromVector(type, {false, true}, {GetFloat(0.5), nan_value}, &a); - ArrayFromVector(type, {false, true}, {GetFloat(0.5), nan_value}, &b); + ArrayFromVector(type, {false, true}, {RealToCType(0.5), nan_value}, &a); + ArrayFromVector(type, {false, true}, {RealToCType(0.5), nan_value}, &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_TRUE(a->Equals(b, EqualOptions().nans_equal(true))); @@ -2180,9 +2160,9 @@ void CheckFloatingNanEquality() { ASSERT_TRUE(b->RangeEquals(a, 0, 1, 0)); // NaN != non-NaN - ArrayFromVector(type, {false, true}, {GetFloat(0.5), nan_value}, &a); - ArrayFromVector(type, {false, true}, {GetFloat(0.5), GetFloat(0.0)}, - &b); + ArrayFromVector(type, {false, true}, {RealToCType(0.5), nan_value}, &a); + ArrayFromVector(type, {false, true}, + {RealToCType(0.5), RealToCType(0.0)}, &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_FALSE(a->Equals(b, EqualOptions().nans_equal(true))); @@ -2206,14 +2186,14 @@ void CheckFloatingInfinityEquality() { std::shared_ptr a, b; std::shared_ptr type = TypeTraits::type_singleton(); - const auto infinity = GetFloat(std::numeric_limits::infinity()); + const auto infinity = RealToCType(std::numeric_limits::infinity()); for (auto nans_equal : {false, true}) { // Infinity in a null entry ArrayFromVector(type, {true, false}, - {GetFloat(0.5), GetFloat(infinity)}, &a); + {RealToCType(0.5), RealToCType(infinity)}, &a); ArrayFromVector(type, {true, false}, - {GetFloat(0.5), GetFloat(-infinity)}, &b); + {RealToCType(0.5), RealToCType(-infinity)}, &b); ASSERT_TRUE(a->Equals(b)); ASSERT_TRUE(b->Equals(a)); ASSERT_TRUE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); @@ -2225,9 +2205,9 @@ void CheckFloatingInfinityEquality() { // Infinity in a valid entry ArrayFromVector(type, {false, true}, - {GetFloat(0.5), GetFloat(infinity)}, &a); + {RealToCType(0.5), RealToCType(infinity)}, &a); ArrayFromVector(type, {false, true}, - {GetFloat(0.5), GetFloat(infinity)}, &b); + {RealToCType(0.5), RealToCType(infinity)}, &b); ASSERT_TRUE(a->Equals(b)); ASSERT_TRUE(b->Equals(a)); ASSERT_TRUE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); @@ -2245,9 +2225,9 @@ void CheckFloatingInfinityEquality() { // Infinity != non-infinity ArrayFromVector(type, {false, true}, - {GetFloat(0.5), GetFloat(-infinity)}, &a); - ArrayFromVector(type, {false, true}, {GetFloat(0.5), GetFloat(0.0)}, - &b); + {RealToCType(0.5), RealToCType(-infinity)}, &a); + ArrayFromVector(type, {false, true}, + {RealToCType(0.5), RealToCType(0.0)}, &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_FALSE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); @@ -2256,9 +2236,9 @@ void CheckFloatingInfinityEquality() { ASSERT_FALSE(b->ApproxEquals(a, EqualOptions().atol(1e-5).nans_equal(nans_equal))); // Infinity != Negative infinity ArrayFromVector(type, {true, true}, - {GetFloat(0.5), GetFloat(-infinity)}, &a); + {RealToCType(0.5), RealToCType(-infinity)}, &a); ArrayFromVector(type, {true, true}, - {GetFloat(0.5), GetFloat(infinity)}, &b); + {RealToCType(0.5), RealToCType(infinity)}, &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_FALSE(a->ApproxEquals(b)); @@ -2281,10 +2261,10 @@ void CheckFloatingZeroEquality() { std::shared_ptr a, b; std::shared_ptr type = TypeTraits::type_singleton(); - ArrayFromVector(type, {true, false}, {GetFloat(0.0), GetFloat(1.0)}, - &a); - ArrayFromVector(type, {true, false}, {GetFloat(0.0), GetFloat(1.0)}, - &b); + ArrayFromVector(type, {true, false}, + {RealToCType(0.0), RealToCType(1.0)}, &a); + ArrayFromVector(type, {true, false}, + {RealToCType(0.0), RealToCType(1.0)}, &b); ASSERT_TRUE(a->Equals(b)); ASSERT_TRUE(b->Equals(a)); for (auto nans_equal : {false, true}) { @@ -2300,10 +2280,10 @@ void CheckFloatingZeroEquality() { } } - ArrayFromVector(type, {true, false}, {GetFloat(0.0), GetFloat(1.0)}, - &a); - ArrayFromVector(type, {true, false}, {GetFloat(-0.0), GetFloat(1.0)}, - &b); + ArrayFromVector(type, {true, false}, + {RealToCType(0.0), RealToCType(1.0)}, &a); + ArrayFromVector(type, {true, false}, + {RealToCType(-0.0), RealToCType(1.0)}, &b); for (auto nans_equal : {false, true}) { auto opts = EqualOptions().nans_equal(nans_equal); ASSERT_TRUE(a->Equals(b, opts)); diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index e289e455640c..4a240de6a0f1 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -299,26 +299,6 @@ TYPED_TEST(TestNumericScalar, MakeScalar) { AssertParseScalar(type, "3", ScalarType(3)); } -template -auto GetFloat(double d) { - if constexpr (std::is_same_v) { - const auto h = Float16::FromDouble(d); - // Double check that nan/inf/sign are preserved - if (std::isnan(d)) { - EXPECT_TRUE(h.is_nan()); - } - if (std::isinf(d)) { - EXPECT_TRUE(h.is_infinity()); - } - if (std::signbit(d)) { - EXPECT_TRUE(h.signbit()); - } - return h.bits(); - } else { - return static_cast(d); - } -} - template class TestRealScalar : public ::testing::Test { public: @@ -328,21 +308,22 @@ class TestRealScalar : public ::testing::Test { void SetUp() { type_ = TypeTraits::type_singleton(); - scalar_val_ = std::make_shared(GetFloat(1)); + scalar_val_ = std::make_shared(RealToCType(1)); ASSERT_TRUE(scalar_val_->is_valid); - scalar_other_ = std::make_shared(GetFloat(1.1)); + scalar_other_ = std::make_shared(RealToCType(1.1)); ASSERT_TRUE(scalar_other_->is_valid); - scalar_zero_ = std::make_shared(GetFloat(0.0)); - scalar_other_zero_ = std::make_shared(GetFloat(0.0)); - scalar_neg_zero_ = std::make_shared(GetFloat(-0.0)); + scalar_zero_ = std::make_shared(RealToCType(0.0)); + scalar_other_zero_ = std::make_shared(RealToCType(0.0)); + scalar_neg_zero_ = std::make_shared(RealToCType(-0.0)); - const CType nan_value = GetFloat(std::numeric_limits::quiet_NaN()); + const CType nan_value = RealToCType(std::numeric_limits::quiet_NaN()); scalar_nan_ = std::make_shared(nan_value); ASSERT_TRUE(scalar_nan_->is_valid); - const CType other_nan_value = GetFloat(std::numeric_limits::quiet_NaN()); + const CType other_nan_value = + RealToCType(std::numeric_limits::quiet_NaN()); scalar_other_nan_ = std::make_shared(other_nan_value); ASSERT_TRUE(scalar_other_nan_->is_valid); } diff --git a/cpp/src/arrow/testing/gtest_util.h b/cpp/src/arrow/testing/gtest_util.h index 62bf907a2d89..5a467c46eb83 100644 --- a/cpp/src/arrow/testing/gtest_util.h +++ b/cpp/src/arrow/testing/gtest_util.h @@ -18,6 +18,7 @@ #pragma once #include +#include #include #include #include @@ -39,6 +40,7 @@ #include "arrow/testing/visibility.h" #include "arrow/type_fwd.h" #include "arrow/type_traits.h" +#include "arrow/util/float16.h" #include "arrow/util/macros.h" #include "arrow/util/string_util.h" #include "arrow/util/type_fwd.h" @@ -572,4 +574,21 @@ ARROW_TESTING_EXPORT std::shared_ptr UnalignBuffers(const ArrayData& /// This method does not recurse into the dictionary or children ARROW_TESTING_EXPORT std::shared_ptr UnalignBuffers(const Array& array); +/// \brief Convert a floating point value to it's corresponding C Type +/// +/// Useful when testing HalfFloat (uint16_t) values alongside native floating point types +template +auto RealToCType(double d) { + if constexpr (is_half_float_type::value) { + const auto h = util::Float16::FromDouble(d); + // Double check that nan/inf/sign are preserved + EXPECT_EQ(h.is_nan(), std::isnan(d)); + EXPECT_EQ(h.is_infinity(), std::isinf(d)); + EXPECT_EQ(h.signbit(), std::signbit(d)); + return h.bits(); + } else { + return static_cast(d); + } +} + } // namespace arrow From 7a730c5d0f545b8ae8a5e1953d6e7d8fa04549c3 Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Thu, 10 Jul 2025 19:38:39 -0400 Subject: [PATCH 06/15] Enable constructing HalfFloatScalar from Float16 --- cpp/src/arrow/scalar.h | 19 +++++++++++++++++++ cpp/src/arrow/type_fwd.h | 1 + cpp/src/arrow/type_traits.h | 5 +++++ 3 files changed, 25 insertions(+) diff --git a/cpp/src/arrow/scalar.h b/cpp/src/arrow/scalar.h index 7ef37301203b..81bec9621119 100644 --- a/cpp/src/arrow/scalar.h +++ b/cpp/src/arrow/scalar.h @@ -37,6 +37,7 @@ #include "arrow/type_traits.h" #include "arrow/util/compare.h" #include "arrow/util/decimal.h" +#include "arrow/util/float16.h" #include "arrow/util/visibility.h" #include "arrow/visit_type_inline.h" @@ -245,6 +246,12 @@ struct ARROW_EXPORT UInt64Scalar : public NumericScalar { struct ARROW_EXPORT HalfFloatScalar : public NumericScalar { using NumericScalar::NumericScalar; + + explicit HalfFloatScalar(util::Float16 value) + : NumericScalar(value.bits(), float16()) {} + + HalfFloatScalar(util::Float16 value, std::shared_ptr type) + : NumericScalar(value.bits(), std::move(type)) {} }; struct ARROW_EXPORT FloatScalar : public NumericScalar { @@ -969,6 +976,18 @@ struct MakeScalarImpl { return Status::OK(); } + // This isn't captured by the generic case above because `util::Float16` isn't implicity + // convertible to `uint16_t` (HalfFloat's ValueType) + template + std::enable_if_t, util::Float16> && + is_half_float_type::value, + Status> + Visit(const T& t) { + out_ = std::make_shared(static_cast(value_), + std::move(type_)); + return Status::OK(); + } + Status Visit(const ExtensionType& t) { ARROW_ASSIGN_OR_RAISE(auto storage, MakeScalar(t.storage_type(), static_cast(value_))); diff --git a/cpp/src/arrow/type_fwd.h b/cpp/src/arrow/type_fwd.h index dc290cd327ae..be26c40dc1f4 100644 --- a/cpp/src/arrow/type_fwd.h +++ b/cpp/src/arrow/type_fwd.h @@ -46,6 +46,7 @@ class Future; namespace util { class Codec; class CodecOptions; +class Float16; } // namespace util class Buffer; diff --git a/cpp/src/arrow/type_traits.h b/cpp/src/arrow/type_traits.h index 90c110a96b01..1b7a02e1085a 100644 --- a/cpp/src/arrow/type_traits.h +++ b/cpp/src/arrow/type_traits.h @@ -316,6 +316,11 @@ struct TypeTraits { static inline std::shared_ptr type_singleton() { return float16(); } }; +template <> +struct CTypeTraits : public TypeTraits { + using ArrowType = HalfFloatType; +}; + template <> struct TypeTraits { using ArrayType = Decimal32Array; From 08d45d17180ed9d10c4e2eae2e4b8db2d13d85e4 Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Thu, 10 Jul 2025 19:40:37 -0400 Subject: [PATCH 07/15] Test HalfFloat in TestNumericScalar --- cpp/src/arrow/scalar_test.cc | 72 +++++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 22 deletions(-) diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 4a240de6a0f1..0620af5cfc3f 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -203,22 +203,42 @@ TEST(TestScalar, IdentityCast) { */ } +template +struct NumericHelper { + using ArgType = typename ARROW_TYPE::c_type; + template + static T Arg(T v) { + return v; + } +}; +template <> +struct NumericHelper { + using ArgType = Float16; + template + static ArgType Arg(T v) { + return static_cast(v); + } +}; + template class TestNumericScalar : public ::testing::Test { public: TestNumericScalar() = default; }; -TYPED_TEST_SUITE(TestNumericScalar, NumericArrowTypes); +using NumericArrowTypesPlusHalfFloat = + testing::Types; +TYPED_TEST_SUITE(TestNumericScalar, NumericArrowTypesPlusHalfFloat); TYPED_TEST(TestNumericScalar, Basics) { - using T = typename TypeParam::c_type; + using Helper = NumericHelper; + using T = typename Helper::ArgType; using ScalarType = typename TypeTraits::ScalarType; T value = static_cast(1); auto scalar_val = std::make_shared(value); - ASSERT_EQ(value, scalar_val->value); ASSERT_TRUE(scalar_val->is_valid); ASSERT_OK(scalar_val->ValidateFull()); @@ -229,8 +249,15 @@ TYPED_TEST(TestNumericScalar, Basics) { auto scalar_other = std::make_shared(other_value); ASSERT_NE(*scalar_other, *scalar_val); - scalar_val->value = other_value; - ASSERT_EQ(other_value, scalar_val->value); + if constexpr (is_half_float_type::value) { + ASSERT_EQ(value, Float16::FromBits(scalar_val->value)); + scalar_val->value = other_value.bits(); + ASSERT_EQ(other_value, Float16::FromBits(scalar_val->value)); + } else { + ASSERT_EQ(value, scalar_val->value); + scalar_val->value = other_value; + ASSERT_EQ(other_value, scalar_val->value); + } ASSERT_EQ(*scalar_other, *scalar_val); ScalarType stack_val; @@ -257,46 +284,47 @@ TYPED_TEST(TestNumericScalar, Basics) { ASSERT_OK(two->ValidateFull()); ASSERT_TRUE(null->Equals(*null_value)); - ASSERT_TRUE(one->Equals(ScalarType(1))); - ASSERT_FALSE(one->Equals(ScalarType(2))); - ASSERT_TRUE(two->Equals(ScalarType(2))); - ASSERT_FALSE(two->Equals(ScalarType(3))); + ASSERT_TRUE(one->Equals(ScalarType(Helper::Arg(1)))); + ASSERT_FALSE(one->Equals(ScalarType(Helper::Arg(2)))); + ASSERT_TRUE(two->Equals(ScalarType(Helper::Arg(2)))); + ASSERT_FALSE(two->Equals(ScalarType(Helper::Arg(3)))); ASSERT_TRUE(null->ApproxEquals(*null_value)); - ASSERT_TRUE(one->ApproxEquals(ScalarType(1))); - ASSERT_FALSE(one->ApproxEquals(ScalarType(2))); - ASSERT_TRUE(two->ApproxEquals(ScalarType(2))); - ASSERT_FALSE(two->ApproxEquals(ScalarType(3))); + ASSERT_TRUE(one->ApproxEquals(ScalarType(Helper::Arg(1)))); + ASSERT_FALSE(one->ApproxEquals(ScalarType(Helper::Arg(2)))); + ASSERT_TRUE(two->ApproxEquals(ScalarType(Helper::Arg(2)))); + ASSERT_FALSE(two->ApproxEquals(ScalarType(Helper::Arg(3)))); } TYPED_TEST(TestNumericScalar, Hashing) { - using T = typename TypeParam::c_type; + using T = typename NumericHelper::ArgType; using ScalarType = typename TypeTraits::ScalarType; std::unordered_set, Scalar::Hash, Scalar::PtrsEqual> set; set.emplace(std::make_shared()); - for (T i = 0; i < 10; ++i) { - set.emplace(std::make_shared(i)); + for (int i = 0; i < 10; ++i) { + set.emplace(std::make_shared(static_cast(i))); } ASSERT_FALSE(set.emplace(std::make_shared()).second); - for (T i = 0; i < 10; ++i) { - ASSERT_FALSE(set.emplace(std::make_shared(i)).second); + for (int i = 0; i < 10; ++i) { + ASSERT_FALSE(set.emplace(std::make_shared(static_cast(i))).second); } } TYPED_TEST(TestNumericScalar, MakeScalar) { - using T = typename TypeParam::c_type; + using Helper = NumericHelper; + using T = typename Helper::ArgType; using ScalarType = typename TypeTraits::ScalarType; auto type = TypeTraits::type_singleton(); std::shared_ptr three = MakeScalar(static_cast(3)); ASSERT_OK(three->ValidateFull()); - ASSERT_EQ(ScalarType(3), *three); + ASSERT_EQ(ScalarType(Helper::Arg(3)), *three); - AssertMakeScalar(ScalarType(3), type, static_cast(3)); + AssertMakeScalar(ScalarType(Helper::Arg(3)), type, static_cast(3)); - AssertParseScalar(type, "3", ScalarType(3)); + AssertParseScalar(type, "3", ScalarType(Helper::Arg(3))); } template From 8c8ad062fdb9d5a7fc94320a31f32c676a6f7ac2 Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Thu, 10 Jul 2025 19:41:40 -0400 Subject: [PATCH 08/15] Fix params for RandomArrayGenerator::Float16 --- cpp/src/arrow/testing/random.cc | 10 ++++------ cpp/src/arrow/testing/random.h | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/cpp/src/arrow/testing/random.cc b/cpp/src/arrow/testing/random.cc index b6ee93e32ea3..73aca85cb087 100644 --- a/cpp/src/arrow/testing/random.cc +++ b/cpp/src/arrow/testing/random.cc @@ -252,16 +252,14 @@ std::shared_ptr RandomArrayGenerator::Date64(int64_t size, int64_t min, memory_pool); } -std::shared_ptr RandomArrayGenerator::Float16(int64_t size, int16_t min, - int16_t max, double null_probability, +std::shared_ptr RandomArrayGenerator::Float16(int64_t size, uint16_t min, + uint16_t max, + double null_probability, int64_t alignment, MemoryPool* memory_pool) { using OptionType = GenerateOptions, HalfFloatType>; - // FIXME: Not sure why the input min/max are signed when Float16's ctype is uint16_t - uint16_t umin = static_cast(min); - uint16_t umax = static_cast(max); - OptionType options(seed(), umin, umax, null_probability, /*nan_probability=*/0); + OptionType options(seed(), min, max, null_probability, /*nan_probability=*/0); return GenerateNumericArray(size, options, alignment, memory_pool); } diff --git a/cpp/src/arrow/testing/random.h b/cpp/src/arrow/testing/random.h index ad87b1210591..9018553a5689 100644 --- a/cpp/src/arrow/testing/random.h +++ b/cpp/src/arrow/testing/random.h @@ -198,7 +198,7 @@ class ARROW_TESTING_EXPORT RandomArrayGenerator { /// \param[in] memory_pool memory pool to allocate memory from /// /// \return a generated Array - std::shared_ptr Float16(int64_t size, int16_t min, int16_t max, + std::shared_ptr Float16(int64_t size, uint16_t min, uint16_t max, double null_probability = 0, int64_t alignment = kDefaultBufferAlignment, MemoryPool* memory_pool = default_memory_pool()); From e3a3688ec53a65997b25ece99e3354363e536506 Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Thu, 10 Jul 2025 19:46:55 -0400 Subject: [PATCH 09/15] Use is_half_float_type --- cpp/src/arrow/testing/random.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/testing/random.cc b/cpp/src/arrow/testing/random.cc index 73aca85cb087..e3b6d8ed1cc1 100644 --- a/cpp/src/arrow/testing/random.cc +++ b/cpp/src/arrow/testing/random.cc @@ -103,7 +103,7 @@ struct GenerateOptions { pcg32_fast rng(seed_++); DistributionType dist(min_, max_); - if constexpr (std::is_same_v) { + if constexpr (is_half_float_type::value) { // Special handling is required to prevent generating Float16 NaNs std::generate(data, data + n, [&] { Float16 f; From 109fe8c09fe3092c3c6b141ca282df38a2522c38 Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Thu, 10 Jul 2025 20:40:03 -0400 Subject: [PATCH 10/15] Fix MSVC warnings --- cpp/src/arrow/scalar_test.cc | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 0620af5cfc3f..85ca84e2e9db 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -206,18 +206,10 @@ TEST(TestScalar, IdentityCast) { template struct NumericHelper { using ArgType = typename ARROW_TYPE::c_type; - template - static T Arg(T v) { - return v; - } }; template <> struct NumericHelper { using ArgType = Float16; - template - static ArgType Arg(T v) { - return static_cast(v); - } }; template @@ -232,8 +224,7 @@ using NumericArrowTypesPlusHalfFloat = TYPED_TEST_SUITE(TestNumericScalar, NumericArrowTypesPlusHalfFloat); TYPED_TEST(TestNumericScalar, Basics) { - using Helper = NumericHelper; - using T = typename Helper::ArgType; + using T = typename NumericHelper::ArgType; using ScalarType = typename TypeTraits::ScalarType; T value = static_cast(1); @@ -284,16 +275,16 @@ TYPED_TEST(TestNumericScalar, Basics) { ASSERT_OK(two->ValidateFull()); ASSERT_TRUE(null->Equals(*null_value)); - ASSERT_TRUE(one->Equals(ScalarType(Helper::Arg(1)))); - ASSERT_FALSE(one->Equals(ScalarType(Helper::Arg(2)))); - ASSERT_TRUE(two->Equals(ScalarType(Helper::Arg(2)))); - ASSERT_FALSE(two->Equals(ScalarType(Helper::Arg(3)))); + ASSERT_TRUE(one->Equals(ScalarType(static_cast(1)))); + ASSERT_FALSE(one->Equals(ScalarType(static_cast(2)))); + ASSERT_TRUE(two->Equals(ScalarType(static_cast(2)))); + ASSERT_FALSE(two->Equals(ScalarType(static_cast(3)))); ASSERT_TRUE(null->ApproxEquals(*null_value)); - ASSERT_TRUE(one->ApproxEquals(ScalarType(Helper::Arg(1)))); - ASSERT_FALSE(one->ApproxEquals(ScalarType(Helper::Arg(2)))); - ASSERT_TRUE(two->ApproxEquals(ScalarType(Helper::Arg(2)))); - ASSERT_FALSE(two->ApproxEquals(ScalarType(Helper::Arg(3)))); + ASSERT_TRUE(one->ApproxEquals(ScalarType(static_cast(1)))); + ASSERT_FALSE(one->ApproxEquals(ScalarType(static_cast(2)))); + ASSERT_TRUE(two->ApproxEquals(ScalarType(static_cast(2)))); + ASSERT_FALSE(two->ApproxEquals(ScalarType(static_cast(3)))); } TYPED_TEST(TestNumericScalar, Hashing) { @@ -313,18 +304,17 @@ TYPED_TEST(TestNumericScalar, Hashing) { } TYPED_TEST(TestNumericScalar, MakeScalar) { - using Helper = NumericHelper; - using T = typename Helper::ArgType; + using T = typename NumericHelper::ArgType; using ScalarType = typename TypeTraits::ScalarType; auto type = TypeTraits::type_singleton(); std::shared_ptr three = MakeScalar(static_cast(3)); ASSERT_OK(three->ValidateFull()); - ASSERT_EQ(ScalarType(Helper::Arg(3)), *three); + ASSERT_EQ(ScalarType(static_cast(3)), *three); - AssertMakeScalar(ScalarType(Helper::Arg(3)), type, static_cast(3)); + AssertMakeScalar(ScalarType(static_cast(3)), type, static_cast(3)); - AssertParseScalar(type, "3", ScalarType(Helper::Arg(3))); + AssertParseScalar(type, "3", ScalarType(static_cast(3))); } template From 4e69d1cc7d69e8330030fc95d2e55f977b6bd54e Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Mon, 1 Sep 2025 20:11:00 -0400 Subject: [PATCH 11/15] Improve random HalfFloat generation --- cpp/src/arrow/testing/random.cc | 121 +++++++++++++++++---------- cpp/src/arrow/testing/random.h | 28 ++++++- cpp/src/arrow/testing/random_test.cc | 30 ++++++- 3 files changed, 129 insertions(+), 50 deletions(-) diff --git a/cpp/src/arrow/testing/random.cc b/cpp/src/arrow/testing/random.cc index e3b6d8ed1cc1..f3653fde49cd 100644 --- a/cpp/src/arrow/testing/random.cc +++ b/cpp/src/arrow/testing/random.cc @@ -61,61 +61,79 @@ namespace random { namespace { -template +template +struct GeneratorFactory { + GeneratorFactory(ValueType min, ValueType max) : min_(min), max_(max) {} + + auto operator()(pcg32_fast* rng) const { + return [dist = DistributionType(min_, max_), rng]() mutable { + return static_cast(dist(*rng)); + }; + } + + private: + ValueType min_; + ValueType max_; +}; + +template +struct GeneratorFactory { + GeneratorFactory(Float16 min, Float16 max) : min_(min.ToFloat()), max_(max.ToFloat()) {} + + auto operator()(pcg32_fast* rng) const { + return [dist = DistributionType(min_, max_), rng]() mutable { + return Float16(dist(*rng)).bits(); + }; + } + + private: + float min_; + float max_; +}; + +template struct GenerateOptions { + static constexpr bool kIsHalfFloat = std::is_same_v; + using PhysicalType = std::conditional_t; + using FactoryType = GeneratorFactory; + GenerateOptions(SeedType seed, ValueType min, ValueType max, double probability, double nan_probability = 0.0) - : min_(min), - max_(max), + : generator_factory_(FactoryType(min, max)), seed_(seed), probability_(probability), nan_probability_(nan_probability) {} void GenerateData(uint8_t* buffer, size_t n) { - GenerateTypedData(reinterpret_cast(buffer), n); + GenerateTypedData(reinterpret_cast(buffer), n); } template - typename std::enable_if::value>::type GenerateTypedData( - V* data, size_t n) { + typename std::enable_if && !kIsHalfFloat>::type + GenerateTypedData(V* data, size_t n) { GenerateTypedDataNoNan(data, n); } template - typename std::enable_if::value>::type GenerateTypedData( - V* data, size_t n) { + typename std::enable_if || kIsHalfFloat>::type + GenerateTypedData(V* data, size_t n) { if (nan_probability_ == 0.0) { GenerateTypedDataNoNan(data, n); return; } pcg32_fast rng(seed_++); - DistributionType dist(min_, max_); + auto gen = generator_factory_(&rng); ::arrow::random::bernoulli_distribution nan_dist(nan_probability_); - const ValueType nan_value = std::numeric_limits::quiet_NaN(); + const PhysicalType nan_value = get_nan(); - // A static cast is required due to the int16 -> int8 handling. - std::generate(data, data + n, [&] { - return nan_dist(rng) ? nan_value : static_cast(dist(rng)); - }); + std::generate(data, data + n, [&] { return nan_dist(rng) ? nan_value : gen(); }); } - void GenerateTypedDataNoNan(ValueType* data, size_t n) { + void GenerateTypedDataNoNan(PhysicalType* data, size_t n) { pcg32_fast rng(seed_++); - DistributionType dist(min_, max_); - - if constexpr (is_half_float_type::value) { - // Special handling is required to prevent generating Float16 NaNs - std::generate(data, data + n, [&] { - Float16 f; - do { - f = Float16::FromBits(static_cast(dist(rng))); - } while (f.is_nan()); - return f.bits(); - }); - } else { - // A static cast is required due to the int16 -> int8 handling. - std::generate(data, data + n, [&] { return static_cast(dist(rng)); }); - } + auto gen = generator_factory_(&rng); + + std::generate(data, data + n, [&] { return gen(); }); } void GenerateBitmap(uint8_t* buffer, size_t n, int64_t* null_count) { @@ -134,8 +152,15 @@ struct GenerateOptions { if (null_count != nullptr) *null_count = count; } - ValueType min_; - ValueType max_; + static constexpr PhysicalType get_nan() { + if constexpr (kIsHalfFloat) { + return std::numeric_limits::quiet_NaN().bits(); + } else { + return std::numeric_limits::quiet_NaN(); + } + } + + FactoryType generator_factory_; SeedType seed_; double probability_; double nan_probability_; @@ -257,9 +282,16 @@ std::shared_ptr RandomArrayGenerator::Float16(int64_t size, uint16_t min, double null_probability, int64_t alignment, MemoryPool* memory_pool) { + return this->Float16(size, Float16::FromBits(min), Float16::FromBits(max), + null_probability, /*nan_probability=*/0, alignment, memory_pool); +} + +std::shared_ptr RandomArrayGenerator::Float16( + int64_t size, util::Float16 min, util::Float16 max, double null_probability, + double nan_probability, int64_t alignment, MemoryPool* memory_pool) { using OptionType = - GenerateOptions, HalfFloatType>; - OptionType options(seed(), min, max, null_probability, /*nan_probability=*/0); + GenerateOptions>; + OptionType options(seed(), min, max, null_probability, nan_probability); return GenerateNumericArray(size, options, alignment, memory_pool); } @@ -1116,20 +1148,19 @@ std::shared_ptr RandomArrayGenerator::ArrayOf(const Field& field, int64_t GENERATE_FLOATING_CASE(DoubleType, Float64); case Type::type::HALF_FLOAT: { - using CType = HalfFloatType::c_type; - const CType min_value = - GetMetadata(field.metadata().get(), "min", - std::numeric_limits<::arrow::util::Float16>::min().bits()); - const CType max_value = - GetMetadata(field.metadata().get(), "max", - std::numeric_limits<::arrow::util::Float16>::max().bits()); + using ValueType = util::Float16; + const ValueType min_value = GetMetadata( + field.metadata().get(), "min", std::numeric_limits::min()); + const ValueType max_value = GetMetadata( + field.metadata().get(), "max", std::numeric_limits::max()); const double nan_probability = GetMetadata(field.metadata().get(), "nan_probability", 0); - VALIDATE_MIN_MAX(Float16::FromBits(min_value), Float16::FromBits(max_value)); + VALIDATE_MIN_MAX(min_value, max_value); VALIDATE_RANGE(nan_probability, 0.0, 1.0); - // TODO: New interface to allow passing `nan_probability` - return Float16(length, min_value, max_value, null_probability, alignment, - memory_pool); + ARROW_LOG(INFO) << "min = " << min_value.ToFloat(); + ARROW_LOG(INFO) << "max = " << max_value.ToFloat(); + return Float16(length, min_value, max_value, null_probability, nan_probability, + alignment, memory_pool); } case Type::type::STRING: diff --git a/cpp/src/arrow/testing/random.h b/cpp/src/arrow/testing/random.h index 9018553a5689..d9122915a092 100644 --- a/cpp/src/arrow/testing/random.h +++ b/cpp/src/arrow/testing/random.h @@ -28,6 +28,7 @@ #include "arrow/testing/uniform_real.h" #include "arrow/testing/visibility.h" #include "arrow/type.h" +#include "arrow/util/float16.h" namespace arrow { @@ -198,11 +199,33 @@ class ARROW_TESTING_EXPORT RandomArrayGenerator { /// \param[in] memory_pool memory pool to allocate memory from /// /// \return a generated Array + /// + /// \deprecated Deprecated in 22.0.0. Use the other Float16() method that accepts + /// nan_probability as a parameter + ARROW_DEPRECATED( + "Deprecated in 22.0.0. Use the other Float16() method that accepts nan_probability " + "as a parameter") std::shared_ptr Float16(int64_t size, uint16_t min, uint16_t max, double null_probability = 0, int64_t alignment = kDefaultBufferAlignment, MemoryPool* memory_pool = default_memory_pool()); + /// \brief Generate a random HalfFloatArray + /// + /// \param[in] size the size of the array to generate + /// \param[in] min the lower bound of the uniform distribution + /// \param[in] max the upper bound of the uniform distribution + /// \param[in] null_probability the probability of a value being null + /// \param[in] nan_probability the probability of a value being NaN + /// \param[in] alignment alignment for memory allocations (in bytes) + /// \param[in] memory_pool memory pool to allocate memory from + /// + /// \return a generated Array + std::shared_ptr Float16(int64_t size, util::Float16 min, util::Float16 max, + double null_probability = 0, double nan_probability = 0, + int64_t alignment = kDefaultBufferAlignment, + MemoryPool* memory_pool = default_memory_pool()); + /// \brief Generate a random FloatArray /// /// \param[in] size the size of the array to generate @@ -281,8 +304,9 @@ class ARROW_TESTING_EXPORT RandomArrayGenerator { return Int64(size, static_cast(min), static_cast(max), null_probability, alignment, memory_pool); case Type::HALF_FLOAT: - return Float16(size, static_cast(min), static_cast(max), - null_probability, alignment, memory_pool); + return Float16(size, util::Float16::FromBits(static_cast(min)), + util::Float16::FromBits(static_cast(max)), + null_probability, /*nan_probability=*/0, alignment, memory_pool); case Type::FLOAT: return Float32(size, static_cast(min), static_cast(max), null_probability, /*nan_probability=*/0, alignment, memory_pool); diff --git a/cpp/src/arrow/testing/random_test.cc b/cpp/src/arrow/testing/random_test.cc index 6f8621f8e992..279fb6dc91fa 100644 --- a/cpp/src/arrow/testing/random_test.cc +++ b/cpp/src/arrow/testing/random_test.cc @@ -26,12 +26,14 @@ #include "arrow/type_traits.h" #include "arrow/util/checked_cast.h" #include "arrow/util/decimal.h" +#include "arrow/util/float16.h" #include "arrow/util/key_value_metadata.h" #include "arrow/util/pcg_random.h" namespace arrow { using internal::checked_cast; +using util::Float16; namespace random { @@ -242,8 +244,14 @@ TYPED_TEST(RandomNumericArrayTest, GenerateMinMax) { auto array = this->Downcast(batch->column(0)); for (auto slot : *array) { if (!slot.has_value()) continue; - ASSERT_GE(slot, typename TypeParam::c_type(0)); - ASSERT_LE(slot, typename TypeParam::c_type(127)); + if constexpr (is_half_float_type::value) { + const auto f16_slot = Float16::FromBits(*slot); + ASSERT_GE(f16_slot, Float16(0)); + ASSERT_LE(f16_slot, Float16(127)); + } else { + ASSERT_GE(slot, typename TypeParam::c_type(0)); + ASSERT_LE(slot, typename TypeParam::c_type(127)); + } } } @@ -256,7 +264,11 @@ TYPED_TEST(RandomNumericArrayTest, EmptyRange) { auto array = this->Downcast(batch->column(0)); for (auto slot : *array) { if (!slot.has_value()) continue; - ASSERT_EQ(slot, typename TypeParam::c_type(42)); + if constexpr (is_half_float_type::value) { + ASSERT_EQ(Float16::FromBits(*slot), Float16(42)); + } else { + ASSERT_EQ(slot, typename TypeParam::c_type(42)); + } } } @@ -359,6 +371,18 @@ TEST(TypeSpecificTests, DictionaryValues) { ASSERT_EQ(16, array->dictionary()->length()); } +TEST(TypeSpecificTests, Float16Nan) { + auto field = arrow::field("float16", float16(), + key_value_metadata({{"nan_probability", "1.0"}})); + auto base_array = GenerateArray(*field, kExpectedLength, 0xDEADBEEF); + AssertTypeEqual(field->type(), base_array->type()); + auto array = internal::checked_pointer_cast>(base_array); + ASSERT_OK(array->ValidateFull()); + for (const auto& value : *array) { + ASSERT_TRUE(!value.has_value() || Float16::FromBits(*value).is_nan()); + } +} + TEST(TypeSpecificTests, Float32Nan) { auto field = arrow::field("float32", float32(), key_value_metadata({{"nan_probability", "1.0"}})); From 2e53f4a96434a955839cdfe28d25af2168457d10 Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Mon, 1 Sep 2025 20:12:34 -0400 Subject: [PATCH 12/15] Address additional review points --- cpp/src/arrow/scalar.h | 2 +- cpp/src/arrow/scalar_test.cc | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/scalar.h b/cpp/src/arrow/scalar.h index 81bec9621119..b96d930a444c 100644 --- a/cpp/src/arrow/scalar.h +++ b/cpp/src/arrow/scalar.h @@ -979,7 +979,7 @@ struct MakeScalarImpl { // This isn't captured by the generic case above because `util::Float16` isn't implicity // convertible to `uint16_t` (HalfFloat's ValueType) template - std::enable_if_t, util::Float16> && + std::enable_if_t, util::Float16> && is_half_float_type::value, Status> Visit(const T& t) { diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 85ca84e2e9db..39e65425f581 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -230,6 +230,11 @@ TYPED_TEST(TestNumericScalar, Basics) { T value = static_cast(1); auto scalar_val = std::make_shared(value); + if constexpr (is_half_float_type::value) { + ASSERT_EQ(value, Float16::FromBits(scalar_val->value)); + } else { + ASSERT_EQ(value, scalar_val->value); + } ASSERT_TRUE(scalar_val->is_valid); ASSERT_OK(scalar_val->ValidateFull()); @@ -241,11 +246,9 @@ TYPED_TEST(TestNumericScalar, Basics) { ASSERT_NE(*scalar_other, *scalar_val); if constexpr (is_half_float_type::value) { - ASSERT_EQ(value, Float16::FromBits(scalar_val->value)); scalar_val->value = other_value.bits(); ASSERT_EQ(other_value, Float16::FromBits(scalar_val->value)); } else { - ASSERT_EQ(value, scalar_val->value); scalar_val->value = other_value; ASSERT_EQ(other_value, scalar_val->value); } @@ -294,7 +297,7 @@ TYPED_TEST(TestNumericScalar, Hashing) { std::unordered_set, Scalar::Hash, Scalar::PtrsEqual> set; set.emplace(std::make_shared()); for (int i = 0; i < 10; ++i) { - set.emplace(std::make_shared(static_cast(i))); + ASSERT_TRUE(set.emplace(std::make_shared(static_cast(i))).second); } ASSERT_FALSE(set.emplace(std::make_shared()).second); From d038d96d34a0b50d770a6c7eeb7e3f4be2d0dfee Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Mon, 1 Sep 2025 21:20:28 -0400 Subject: [PATCH 13/15] Remove RealToCType Since github.com/apache/arrow/pull/46981, HalfFloatBuilder now accepts Float16 values, making RealToCType's usage unnecessary in several places. --- cpp/src/arrow/array/array_test.cc | 59 +++++++++++++----------------- cpp/src/arrow/scalar_test.cc | 18 ++++----- cpp/src/arrow/testing/gtest_util.h | 19 ---------- 3 files changed, 35 insertions(+), 61 deletions(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index 8c315faeba49..01cad6c1f168 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -2122,14 +2122,16 @@ void CheckSliceApproxEquals() { template void CheckFloatingNanEquality() { + using V = + std::conditional_t::value, Float16, typename TYPE::c_type>; std::shared_ptr a, b; std::shared_ptr type = TypeTraits::type_singleton(); - const auto nan_value = RealToCType(NAN); + const auto nan_value = std::numeric_limits::quiet_NaN(); // NaN in a null entry - ArrayFromVector(type, {true, false}, {RealToCType(0.5), nan_value}, &a); - ArrayFromVector(type, {true, false}, {RealToCType(0.5), nan_value}, &b); + ArrayFromVector(type, {true, false}, {V(0.5), nan_value}, &a); + ArrayFromVector(type, {true, false}, {V(0.5), nan_value}, &b); ASSERT_TRUE(a->Equals(b)); ASSERT_TRUE(b->Equals(a)); ASSERT_TRUE(a->ApproxEquals(b)); @@ -2140,8 +2142,8 @@ void CheckFloatingNanEquality() { ASSERT_TRUE(b->RangeEquals(a, 1, 2, 1)); // NaN in a valid entry - ArrayFromVector(type, {false, true}, {RealToCType(0.5), nan_value}, &a); - ArrayFromVector(type, {false, true}, {RealToCType(0.5), nan_value}, &b); + ArrayFromVector(type, {false, true}, {V(0.5), nan_value}, &a); + ArrayFromVector(type, {false, true}, {V(0.5), nan_value}, &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_TRUE(a->Equals(b, EqualOptions().nans_equal(true))); @@ -2160,9 +2162,8 @@ void CheckFloatingNanEquality() { ASSERT_TRUE(b->RangeEquals(a, 0, 1, 0)); // NaN != non-NaN - ArrayFromVector(type, {false, true}, {RealToCType(0.5), nan_value}, &a); - ArrayFromVector(type, {false, true}, - {RealToCType(0.5), RealToCType(0.0)}, &b); + ArrayFromVector(type, {false, true}, {V(0.5), nan_value}, &a); + ArrayFromVector(type, {false, true}, {V(0.5), V(0.0)}, &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_FALSE(a->Equals(b, EqualOptions().nans_equal(true))); @@ -2183,17 +2184,17 @@ void CheckFloatingNanEquality() { template void CheckFloatingInfinityEquality() { + using V = + std::conditional_t::value, Float16, typename TYPE::c_type>; std::shared_ptr a, b; std::shared_ptr type = TypeTraits::type_singleton(); - const auto infinity = RealToCType(std::numeric_limits::infinity()); + const auto infinity = std::numeric_limits::infinity(); for (auto nans_equal : {false, true}) { // Infinity in a null entry - ArrayFromVector(type, {true, false}, - {RealToCType(0.5), RealToCType(infinity)}, &a); - ArrayFromVector(type, {true, false}, - {RealToCType(0.5), RealToCType(-infinity)}, &b); + ArrayFromVector(type, {true, false}, {V(0.5), infinity}, &a); + ArrayFromVector(type, {true, false}, {V(0.5), -infinity}, &b); ASSERT_TRUE(a->Equals(b)); ASSERT_TRUE(b->Equals(a)); ASSERT_TRUE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); @@ -2204,10 +2205,8 @@ void CheckFloatingInfinityEquality() { ASSERT_TRUE(b->RangeEquals(a, 1, 2, 1)); // Infinity in a valid entry - ArrayFromVector(type, {false, true}, - {RealToCType(0.5), RealToCType(infinity)}, &a); - ArrayFromVector(type, {false, true}, - {RealToCType(0.5), RealToCType(infinity)}, &b); + ArrayFromVector(type, {false, true}, {V(0.5), infinity}, &a); + ArrayFromVector(type, {false, true}, {V(0.5), infinity}, &b); ASSERT_TRUE(a->Equals(b)); ASSERT_TRUE(b->Equals(a)); ASSERT_TRUE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); @@ -2224,10 +2223,8 @@ void CheckFloatingInfinityEquality() { ASSERT_TRUE(b->RangeEquals(a, 0, 1, 0)); // Infinity != non-infinity - ArrayFromVector(type, {false, true}, - {RealToCType(0.5), RealToCType(-infinity)}, &a); - ArrayFromVector(type, {false, true}, - {RealToCType(0.5), RealToCType(0.0)}, &b); + ArrayFromVector(type, {false, true}, {V(0.5), -infinity}, &a); + ArrayFromVector(type, {false, true}, {V(0.5), V(0.0)}, &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_FALSE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); @@ -2235,10 +2232,8 @@ void CheckFloatingInfinityEquality() { ASSERT_FALSE(a->ApproxEquals(b, EqualOptions().atol(1e-5).nans_equal(nans_equal))); ASSERT_FALSE(b->ApproxEquals(a, EqualOptions().atol(1e-5).nans_equal(nans_equal))); // Infinity != Negative infinity - ArrayFromVector(type, {true, true}, - {RealToCType(0.5), RealToCType(-infinity)}, &a); - ArrayFromVector(type, {true, true}, - {RealToCType(0.5), RealToCType(infinity)}, &b); + ArrayFromVector(type, {true, true}, {V(0.5), -infinity}, &a); + ArrayFromVector(type, {true, true}, {V(0.5), infinity}, &b); ASSERT_FALSE(a->Equals(b)); ASSERT_FALSE(b->Equals(a)); ASSERT_FALSE(a->ApproxEquals(b)); @@ -2258,13 +2253,13 @@ void CheckFloatingInfinityEquality() { template void CheckFloatingZeroEquality() { + using V = + std::conditional_t::value, Float16, typename TYPE::c_type>; std::shared_ptr a, b; std::shared_ptr type = TypeTraits::type_singleton(); - ArrayFromVector(type, {true, false}, - {RealToCType(0.0), RealToCType(1.0)}, &a); - ArrayFromVector(type, {true, false}, - {RealToCType(0.0), RealToCType(1.0)}, &b); + ArrayFromVector(type, {true, false}, {V(0.0), V(1.0)}, &a); + ArrayFromVector(type, {true, false}, {V(0.0), V(1.0)}, &b); ASSERT_TRUE(a->Equals(b)); ASSERT_TRUE(b->Equals(a)); for (auto nans_equal : {false, true}) { @@ -2280,10 +2275,8 @@ void CheckFloatingZeroEquality() { } } - ArrayFromVector(type, {true, false}, - {RealToCType(0.0), RealToCType(1.0)}, &a); - ArrayFromVector(type, {true, false}, - {RealToCType(-0.0), RealToCType(1.0)}, &b); + ArrayFromVector(type, {true, false}, {V(0.0), V(1.0)}, &a); + ArrayFromVector(type, {true, false}, {V(-0.0), V(1.0)}, &b); for (auto nans_equal : {false, true}) { auto opts = EqualOptions().nans_equal(nans_equal); ASSERT_TRUE(a->Equals(b, opts)); diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index 39e65425f581..a89447362b59 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -323,28 +323,28 @@ TYPED_TEST(TestNumericScalar, MakeScalar) { template class TestRealScalar : public ::testing::Test { public: - using CType = typename T::c_type; + using ValueType = + std::conditional_t::value, Float16, typename T::c_type>; using ScalarType = typename TypeTraits::ScalarType; void SetUp() { type_ = TypeTraits::type_singleton(); - scalar_val_ = std::make_shared(RealToCType(1)); + scalar_val_ = std::make_shared(static_cast(1)); ASSERT_TRUE(scalar_val_->is_valid); - scalar_other_ = std::make_shared(RealToCType(1.1)); + scalar_other_ = std::make_shared(static_cast(1.1)); ASSERT_TRUE(scalar_other_->is_valid); - scalar_zero_ = std::make_shared(RealToCType(0.0)); - scalar_other_zero_ = std::make_shared(RealToCType(0.0)); - scalar_neg_zero_ = std::make_shared(RealToCType(-0.0)); + scalar_zero_ = std::make_shared(static_cast(0.0)); + scalar_other_zero_ = std::make_shared(static_cast(0.0)); + scalar_neg_zero_ = std::make_shared(static_cast(-0.0)); - const CType nan_value = RealToCType(std::numeric_limits::quiet_NaN()); + const auto nan_value = std::numeric_limits::quiet_NaN(); scalar_nan_ = std::make_shared(nan_value); ASSERT_TRUE(scalar_nan_->is_valid); - const CType other_nan_value = - RealToCType(std::numeric_limits::quiet_NaN()); + const auto other_nan_value = std::numeric_limits::quiet_NaN(); scalar_other_nan_ = std::make_shared(other_nan_value); ASSERT_TRUE(scalar_other_nan_->is_valid); } diff --git a/cpp/src/arrow/testing/gtest_util.h b/cpp/src/arrow/testing/gtest_util.h index 5a467c46eb83..62bf907a2d89 100644 --- a/cpp/src/arrow/testing/gtest_util.h +++ b/cpp/src/arrow/testing/gtest_util.h @@ -18,7 +18,6 @@ #pragma once #include -#include #include #include #include @@ -40,7 +39,6 @@ #include "arrow/testing/visibility.h" #include "arrow/type_fwd.h" #include "arrow/type_traits.h" -#include "arrow/util/float16.h" #include "arrow/util/macros.h" #include "arrow/util/string_util.h" #include "arrow/util/type_fwd.h" @@ -574,21 +572,4 @@ ARROW_TESTING_EXPORT std::shared_ptr UnalignBuffers(const ArrayData& /// This method does not recurse into the dictionary or children ARROW_TESTING_EXPORT std::shared_ptr UnalignBuffers(const Array& array); -/// \brief Convert a floating point value to it's corresponding C Type -/// -/// Useful when testing HalfFloat (uint16_t) values alongside native floating point types -template -auto RealToCType(double d) { - if constexpr (is_half_float_type::value) { - const auto h = util::Float16::FromDouble(d); - // Double check that nan/inf/sign are preserved - EXPECT_EQ(h.is_nan(), std::isnan(d)); - EXPECT_EQ(h.is_infinity(), std::isinf(d)); - EXPECT_EQ(h.signbit(), std::signbit(d)); - return h.bits(); - } else { - return static_cast(d); - } -} - } // namespace arrow From fb607260d1f526a309215033a1d9ef81a2bc410c Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Wed, 3 Sep 2025 21:02:40 -0400 Subject: [PATCH 14/15] Replace NumericHelper --- cpp/src/arrow/array/array_test.cc | 13 +++++++------ cpp/src/arrow/scalar_test.cc | 20 +++++++------------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index 01cad6c1f168..4db76512d260 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -2120,10 +2120,13 @@ void CheckSliceApproxEquals() { ASSERT_TRUE(slice1->ApproxEquals(slice2)); } +template +using NumericArgType = std::conditional_t::value, Float16, + typename ArrowType::c_type>; + template void CheckFloatingNanEquality() { - using V = - std::conditional_t::value, Float16, typename TYPE::c_type>; + using V = NumericArgType; std::shared_ptr a, b; std::shared_ptr type = TypeTraits::type_singleton(); @@ -2184,8 +2187,7 @@ void CheckFloatingNanEquality() { template void CheckFloatingInfinityEquality() { - using V = - std::conditional_t::value, Float16, typename TYPE::c_type>; + using V = NumericArgType; std::shared_ptr a, b; std::shared_ptr type = TypeTraits::type_singleton(); @@ -2253,8 +2255,7 @@ void CheckFloatingInfinityEquality() { template void CheckFloatingZeroEquality() { - using V = - std::conditional_t::value, Float16, typename TYPE::c_type>; + using V = NumericArgType; std::shared_ptr a, b; std::shared_ptr type = TypeTraits::type_singleton(); diff --git a/cpp/src/arrow/scalar_test.cc b/cpp/src/arrow/scalar_test.cc index a89447362b59..4a34e5d13c20 100644 --- a/cpp/src/arrow/scalar_test.cc +++ b/cpp/src/arrow/scalar_test.cc @@ -203,14 +203,9 @@ TEST(TestScalar, IdentityCast) { */ } -template -struct NumericHelper { - using ArgType = typename ARROW_TYPE::c_type; -}; -template <> -struct NumericHelper { - using ArgType = Float16; -}; +template +using NumericArgType = std::conditional_t::value, Float16, + typename ArrowType::c_type>; template class TestNumericScalar : public ::testing::Test { @@ -224,7 +219,7 @@ using NumericArrowTypesPlusHalfFloat = TYPED_TEST_SUITE(TestNumericScalar, NumericArrowTypesPlusHalfFloat); TYPED_TEST(TestNumericScalar, Basics) { - using T = typename NumericHelper::ArgType; + using T = NumericArgType; using ScalarType = typename TypeTraits::ScalarType; T value = static_cast(1); @@ -291,7 +286,7 @@ TYPED_TEST(TestNumericScalar, Basics) { } TYPED_TEST(TestNumericScalar, Hashing) { - using T = typename NumericHelper::ArgType; + using T = NumericArgType; using ScalarType = typename TypeTraits::ScalarType; std::unordered_set, Scalar::Hash, Scalar::PtrsEqual> set; @@ -307,7 +302,7 @@ TYPED_TEST(TestNumericScalar, Hashing) { } TYPED_TEST(TestNumericScalar, MakeScalar) { - using T = typename NumericHelper::ArgType; + using T = NumericArgType; using ScalarType = typename TypeTraits::ScalarType; auto type = TypeTraits::type_singleton(); @@ -323,8 +318,7 @@ TYPED_TEST(TestNumericScalar, MakeScalar) { template class TestRealScalar : public ::testing::Test { public: - using ValueType = - std::conditional_t::value, Float16, typename T::c_type>; + using ValueType = NumericArgType; using ScalarType = typename TypeTraits::ScalarType; void SetUp() { From 116b9759aade6076d3c75aea6355a2f1ec71e63a Mon Sep 17 00:00:00 2001 From: Benjamin Harkins Date: Thu, 4 Sep 2025 02:23:50 -0400 Subject: [PATCH 15/15] Remove leftover debug logs --- cpp/src/arrow/testing/random.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/cpp/src/arrow/testing/random.cc b/cpp/src/arrow/testing/random.cc index f3653fde49cd..5f95638b7d63 100644 --- a/cpp/src/arrow/testing/random.cc +++ b/cpp/src/arrow/testing/random.cc @@ -1157,8 +1157,6 @@ std::shared_ptr RandomArrayGenerator::ArrayOf(const Field& field, int64_t GetMetadata(field.metadata().get(), "nan_probability", 0); VALIDATE_MIN_MAX(min_value, max_value); VALIDATE_RANGE(nan_probability, 0.0, 1.0); - ARROW_LOG(INFO) << "min = " << min_value.ToFloat(); - ARROW_LOG(INFO) << "max = " << max_value.ToFloat(); return Float16(length, min_value, max_value, null_probability, nan_probability, alignment, memory_pool); }