From 4cb3d097966ebc8734a707068628753ba5c16527 Mon Sep 17 00:00:00 2001 From: Eric Dinse <293818+dinse@users.noreply.github.com> Date: Fri, 27 Jun 2025 11:13:26 -0400 Subject: [PATCH 1/4] Issue #46860 - Adding functions to HalfFloatBuilder that accept Float16 --- cpp/src/arrow/array/array_test.cc | 70 ++++++++++++++++++ cpp/src/arrow/array/builder_primitive.h | 97 ++++++++++++++++++++++++- cpp/src/arrow/type_fwd.h | 7 +- cpp/src/arrow/util/float16.h | 1 + 4 files changed, 173 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index 44ccf9687b77..2324f62db559 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -38,6 +38,7 @@ #include "arrow/array/builder_binary.h" #include "arrow/array/builder_decimal.h" #include "arrow/array/builder_dict.h" +#include "arrow/array/builder_primitive.h" #include "arrow/array/builder_run_end.h" #include "arrow/array/builder_time.h" #include "arrow/array/data.h" @@ -60,6 +61,7 @@ #include "arrow/util/bitmap_builders.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/macros.h" #include "arrow/util/range.h" @@ -72,6 +74,7 @@ namespace arrow { using internal::checked_cast; using internal::checked_pointer_cast; +using util::Float16; class TestArray : public ::testing::Test { public: @@ -4035,4 +4038,71 @@ TYPED_TEST(TestPrimitiveArray, IndexOperator) { } } +class TestHalfFloatBuilder : public ::testing::Test { + public: + void VerifyValue(HalfFloatBuilder& builder, int64_t index, float expected) { + ASSERT_EQ(builder.GetValue(index), Float16(expected).bits()); + ASSERT_EQ(builder.GetValue(index), Float16(expected)); + ASSERT_EQ(builder.GetValue(index), Float16(expected).bits()); + ASSERT_EQ(builder[index], Float16(expected).bits()); + } +}; + +TEST_F(TestHalfFloatBuilder, TestAppend) { + HalfFloatBuilder builder; + ASSERT_OK(builder.Append(Float16(0.0f))); + ASSERT_OK(builder.Append(Float16(1.0f).bits())); + ASSERT_OK(builder.AppendNull()); + ASSERT_OK(builder.Reserve(3)); + builder.UnsafeAppend(Float16(3.0f)); + builder.UnsafeAppend(Float16(4.0f).bits()); + builder.UnsafeAppend(uint16_t{15872}); // 1.5f + + VerifyValue(builder, 0, 0.0f); + VerifyValue(builder, 1, 1.0f); + VerifyValue(builder, 3, 3.0f); + VerifyValue(builder, 4, 4.0f); + VerifyValue(builder, 5, 1.5f); +} + +TEST_F(TestHalfFloatBuilder, TestBulkAppend) { + HalfFloatBuilder builder; + + ASSERT_OK(builder.AppendValues(5, Float16(1.5))); + uint16_t val = Float16(2.0f).bits(); + ASSERT_OK(builder.AppendValues({val, val, val, val}, {0, 1, 0, 1})); + ASSERT_EQ(builder.length(), 9); + for (int i = 0; i < 5; i++) { + VerifyValue(builder, i, 1.5f); + } + ASSERT_OK_AND_ASSIGN(auto array, builder.Finish()); + ArrayData* data = array->data().get(); + ASSERT_EQ(array->null_count(), 2); + ASSERT_EQ(array->length(), 9); + auto comp = ArrayFromJSON(float16(), "[1.5,1.5,1.5,1.5,1.5,null,2,null,2]"); + ASSERT_TRUE(array->Equals(*comp)); + + std::vector vals = {Float16(2.5), Float16(3.5)}; + std::vector is_valid = {true, true}; + ASSERT_OK(builder.AppendValues(vals)); + ASSERT_OK(builder.AppendValues(vals, is_valid)); + ASSERT_OK(builder.AppendValues(vals.data(), vals.size(), is_valid)); + ASSERT_OK(builder.AppendValues(vals.data(), vals.size())); + + for (int i = 0; i < 4; i++) { + VerifyValue(builder, (2 * i), 2.5); + VerifyValue(builder, (2 * i) + 1, 3.5); + } +} + +TEST_F(TestHalfFloatBuilder, TestReinterpretCast) { + std::vector vf{Float16(1.0f), Float16(2.0f), Float16(3.0f)}; + Float16* fdata = vf.data(); + uint16_t* udata = reinterpret_cast(fdata); + + ASSERT_EQ(udata[0], vf[0].bits()); + ASSERT_EQ(udata[1], vf[1].bits()); + ASSERT_EQ(udata[2], vf[2].bits()); +} + } // namespace arrow diff --git a/cpp/src/arrow/array/builder_primitive.h b/cpp/src/arrow/array/builder_primitive.h index be9761fb46b3..944650d54b24 100644 --- a/cpp/src/arrow/array/builder_primitive.h +++ b/cpp/src/arrow/array/builder_primitive.h @@ -26,6 +26,7 @@ #include "arrow/result.h" #include "arrow/type.h" #include "arrow/type_traits.h" +#include "arrow/util/float16.h" namespace arrow { @@ -338,10 +339,104 @@ using Int16Builder = NumericBuilder; using Int32Builder = NumericBuilder; using Int64Builder = NumericBuilder; -using HalfFloatBuilder = NumericBuilder; using FloatBuilder = NumericBuilder; using DoubleBuilder = NumericBuilder; +class ARROW_EXPORT HalfFloatBuilder : public NumericBuilder { + public: + using BaseClass = NumericBuilder; + using Float16 = arrow::util::Float16; + + using BaseClass::Append; + using BaseClass::AppendValues; + using BaseClass::BaseClass; + using BaseClass::GetValue; + using BaseClass::UnsafeAppend; + + /// Scalar append a arrow::util::Float16 + Status Append(const Float16 val) { return Append(val.bits()); } + + /// Scalar append a arrow::util::Float16, without checking for capacity + void UnsafeAppend(const Float16 val) { UnsafeAppend(val.bits()); } + + /// \brief Append a sequence of elements in one shot + /// \param[in] values a contiguous array of arrow::util::Float16 + /// \param[in] length the number of values to append + /// \param[in] valid_bytes an optional sequence of bytes where non-zero + /// indicates a valid (non-null) value + /// \return Status + Status AppendValues(const Float16* values, int64_t length, + const uint8_t* valid_bytes = NULLPTR) { + return BaseClass::AppendValues(reinterpret_cast(values), length, + valid_bytes); + } + + /// \brief Append a sequence of elements in one shot + /// \param[in] values a contiguous array of arrow::util::Float16 + /// \param[in] length the number of values to append + /// \param[in] validity a validity bitmap to copy (may be null) + /// \param[in] offset an offset into the values and validity bitmaps + /// \return Status + Status AppendValues(const Float16* values, int64_t length, const uint8_t* bitmap, + int64_t bitmap_offset) { + return BaseClass::AppendValues(reinterpret_cast(values), length, + bitmap, bitmap_offset); + } + + /// \brief Append a sequence of elements in one shot + /// \param[in] values a contiguous array of arrow::util::Float16 + /// \param[in] length the number of values to append + /// \param[in] is_valid an std::vector indicating valid (1) or null + /// (0). Equal in length to values + /// \return Status + Status AppendValues(const Float16* values, int64_t length, + const std::vector& is_valid) { + return BaseClass::AppendValues(reinterpret_cast(values), length, + is_valid); + } + + /// \brief Append a sequence of elements in one shot + /// \param[in] values an std::vector + /// \param[in] is_valid an std::vector indicating valid (1) or null + /// (0). Equal in length to values + /// \return Status + Status AppendValues(const std::vector& values, + const std::vector& is_valid) { + return AppendValues(values.data(), static_cast(values.size()), is_valid); + } + + /// \brief Append a sequence of elements in one shot + /// \param[in] values an std::vector + /// \return Status + Status AppendValues(const std::vector& values) { + return AppendValues(values.data(), static_cast(values.size())); + } + + /// \brief Append one value many times in one shot + /// \param[in] length the number of values to append + /// \param[in] value a arrow::util::Float16 + Status AppendValues(int64_t length, Float16 value) { + RETURN_NOT_OK(Reserve(length)); + data_builder_.UnsafeAppend(length, value.bits()); + ArrayBuilder::UnsafeSetNotNull(length); + return Status::OK(); + } + + /// \brief Get the value at a certain index + /// \param[in] index the zero-based index + /// @tparam arrow::util::Float16 or value_type (uint16_t) + template + T GetValue(int64_t index) const { + static_assert(std::is_same_v || + std::is_same_v); + if constexpr (std::is_same_v) { + return BaseClass::GetValue(index); + } else { + return Float16::FromBits(BaseClass::GetValue(index)); + } + } +}; + /// @} /// \addtogroup temporal-builders diff --git a/cpp/src/arrow/type_fwd.h b/cpp/src/arrow/type_fwd.h index 5a2fbde0232d..dc290cd327ae 100644 --- a/cpp/src/arrow/type_fwd.h +++ b/cpp/src/arrow/type_fwd.h @@ -242,12 +242,17 @@ _NUMERIC_TYPE_DECL(UInt8) _NUMERIC_TYPE_DECL(UInt16) _NUMERIC_TYPE_DECL(UInt32) _NUMERIC_TYPE_DECL(UInt64) -_NUMERIC_TYPE_DECL(HalfFloat) _NUMERIC_TYPE_DECL(Float) _NUMERIC_TYPE_DECL(Double) #undef _NUMERIC_TYPE_DECL +class HalfFloatType; +using HalfFloatArray = NumericArray; +class HalfFloatBuilder; +struct HalfFloatScalar; +using HalfFloatTensor = NumericTensor; + enum class DateUnit : char { DAY = 0, MILLI = 1 }; class DateType; diff --git a/cpp/src/arrow/util/float16.h b/cpp/src/arrow/util/float16.h index b7455dad5379..285eabc7dd57 100644 --- a/cpp/src/arrow/util/float16.h +++ b/cpp/src/arrow/util/float16.h @@ -176,6 +176,7 @@ class ARROW_EXPORT Float16 { }; static_assert(std::is_trivial_v); +static_assert(std::is_standard_layout_v); } // namespace util } // namespace arrow From c11e7aa243618a6e2802bc9f6ec62f29f56321d7 Mon Sep 17 00:00:00 2001 From: Eric Dinse <293818+dinse@users.noreply.github.com> Date: Mon, 30 Jun 2025 19:49:36 -0400 Subject: [PATCH 2/4] Issue #46860 - Moving HalfFloatBuilder so it shows up in the docs --- cpp/src/arrow/array/array_test.cc | 1 - cpp/src/arrow/array/builder_primitive.h | 36 ++++++++++++------------- docs/source/cpp/api/builder.rst | 3 +++ 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index 2324f62db559..f3ebbc7e0f23 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -4076,7 +4076,6 @@ TEST_F(TestHalfFloatBuilder, TestBulkAppend) { VerifyValue(builder, i, 1.5f); } ASSERT_OK_AND_ASSIGN(auto array, builder.Finish()); - ArrayData* data = array->data().get(); ASSERT_EQ(array->null_count(), 2); ASSERT_EQ(array->length(), 9); auto comp = ArrayFromJSON(float16(), "[1.5,1.5,1.5,1.5,1.5,null,2,null,2]"); diff --git a/cpp/src/arrow/array/builder_primitive.h b/cpp/src/arrow/array/builder_primitive.h index 944650d54b24..c0b0423b1add 100644 --- a/cpp/src/arrow/array/builder_primitive.h +++ b/cpp/src/arrow/array/builder_primitive.h @@ -342,6 +342,22 @@ using Int64Builder = NumericBuilder; using FloatBuilder = NumericBuilder; using DoubleBuilder = NumericBuilder; +/// @} + +/// \addtogroup temporal-builders +/// +/// @{ + +using Date32Builder = NumericBuilder; +using Date64Builder = NumericBuilder; +using Time32Builder = NumericBuilder; +using Time64Builder = NumericBuilder; +using TimestampBuilder = NumericBuilder; +using MonthIntervalBuilder = NumericBuilder; +using DurationBuilder = NumericBuilder; + +/// @} + class ARROW_EXPORT HalfFloatBuilder : public NumericBuilder { public: using BaseClass = NumericBuilder; @@ -374,8 +390,8 @@ class ARROW_EXPORT HalfFloatBuilder : public NumericBuilder { /// \brief Append a sequence of elements in one shot /// \param[in] values a contiguous array of arrow::util::Float16 /// \param[in] length the number of values to append - /// \param[in] validity a validity bitmap to copy (may be null) - /// \param[in] offset an offset into the values and validity bitmaps + /// \param[in] bitmap a validity bitmap to copy (may be null) + /// \param[in] bitmap_offset an offset into the validity bitmap /// \return Status Status AppendValues(const Float16* values, int64_t length, const uint8_t* bitmap, int64_t bitmap_offset) { @@ -437,22 +453,6 @@ class ARROW_EXPORT HalfFloatBuilder : public NumericBuilder { } }; -/// @} - -/// \addtogroup temporal-builders -/// -/// @{ - -using Date32Builder = NumericBuilder; -using Date64Builder = NumericBuilder; -using Time32Builder = NumericBuilder; -using Time64Builder = NumericBuilder; -using TimestampBuilder = NumericBuilder; -using MonthIntervalBuilder = NumericBuilder; -using DurationBuilder = NumericBuilder; - -/// @} - class ARROW_EXPORT BooleanBuilder : public ArrayBuilder, public internal::ArrayBuilderExtraOps { diff --git a/docs/source/cpp/api/builder.rst b/docs/source/cpp/api/builder.rst index 1342ba2655f5..ee3576ec79e8 100644 --- a/docs/source/cpp/api/builder.rst +++ b/docs/source/cpp/api/builder.rst @@ -34,6 +34,9 @@ Primitive .. doxygenclass:: arrow::BooleanBuilder :members: +.. doxygenclass:: arrow::HalfFloatBuilder + :members: + .. doxygengroup:: numeric-builders :content-only: :members: From 659a190e9f7cd69429124accd007bb4934b9204f Mon Sep 17 00:00:00 2001 From: Eric Dinse <293818+dinse@users.noreply.github.com> Date: Mon, 30 Jun 2025 20:40:31 -0400 Subject: [PATCH 3/4] Issue #46860 - Adding a test for the bitmap/offset AppendValues --- cpp/src/arrow/array/array_test.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/array/array_test.cc b/cpp/src/arrow/array/array_test.cc index f3ebbc7e0f23..b3d60e6bfb2a 100644 --- a/cpp/src/arrow/array/array_test.cc +++ b/cpp/src/arrow/array/array_test.cc @@ -4083,12 +4083,14 @@ TEST_F(TestHalfFloatBuilder, TestBulkAppend) { std::vector vals = {Float16(2.5), Float16(3.5)}; std::vector is_valid = {true, true}; + std::vector bitmap = {1, 1}; ASSERT_OK(builder.AppendValues(vals)); ASSERT_OK(builder.AppendValues(vals, is_valid)); ASSERT_OK(builder.AppendValues(vals.data(), vals.size(), is_valid)); ASSERT_OK(builder.AppendValues(vals.data(), vals.size())); + ASSERT_OK(builder.AppendValues(vals.data(), vals.size(), bitmap.data(), 0)); - for (int i = 0; i < 4; i++) { + for (int i = 0; i < 5; i++) { VerifyValue(builder, (2 * i), 2.5); VerifyValue(builder, (2 * i) + 1, 3.5); } From 44549ff01c4c6c9ea959e34d4737b484da1c74af Mon Sep 17 00:00:00 2001 From: Eric Dinse <293818+dinse@users.noreply.github.com> Date: Mon, 30 Jun 2025 21:23:25 -0400 Subject: [PATCH 4/4] Issue #46860 - tweaking grammar --- cpp/src/arrow/array/builder_primitive.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cpp/src/arrow/array/builder_primitive.h b/cpp/src/arrow/array/builder_primitive.h index c0b0423b1add..8dc669b1ea28 100644 --- a/cpp/src/arrow/array/builder_primitive.h +++ b/cpp/src/arrow/array/builder_primitive.h @@ -193,7 +193,7 @@ class NumericBuilder /// \brief Append a sequence of elements in one shot /// \param[in] values a contiguous C array of values /// \param[in] length the number of values to append - /// \param[in] is_valid an std::vector indicating valid (1) or null + /// \param[in] is_valid a std::vector indicating valid (1) or null /// (0). Equal in length to values /// \return Status Status AppendValues(const value_type* values, int64_t length, @@ -402,7 +402,7 @@ class ARROW_EXPORT HalfFloatBuilder : public NumericBuilder { /// \brief Append a sequence of elements in one shot /// \param[in] values a contiguous array of arrow::util::Float16 /// \param[in] length the number of values to append - /// \param[in] is_valid an std::vector indicating valid (1) or null + /// \param[in] is_valid a std::vector indicating valid (1) or null /// (0). Equal in length to values /// \return Status Status AppendValues(const Float16* values, int64_t length, @@ -412,8 +412,8 @@ class ARROW_EXPORT HalfFloatBuilder : public NumericBuilder { } /// \brief Append a sequence of elements in one shot - /// \param[in] values an std::vector - /// \param[in] is_valid an std::vector indicating valid (1) or null + /// \param[in] values a std::vector + /// \param[in] is_valid a std::vector indicating valid (1) or null /// (0). Equal in length to values /// \return Status Status AppendValues(const std::vector& values, @@ -422,7 +422,7 @@ class ARROW_EXPORT HalfFloatBuilder : public NumericBuilder { } /// \brief Append a sequence of elements in one shot - /// \param[in] values an std::vector + /// \param[in] values a std::vector /// \return Status Status AppendValues(const std::vector& values) { return AppendValues(values.data(), static_cast(values.size()));