From 4bb5ff99c8fc72b976a826354e7613ccd2fe94ae Mon Sep 17 00:00:00 2001 From: Paul Mattione Date: Wed, 21 Feb 2024 12:15:39 -0500 Subject: [PATCH 1/5] Compile-time ipow computation with array lookup. Also fix rounding comments. --- cpp/include/cudf/fixed_point/fixed_point.hpp | 60 +++++++++++++------- cpp/include/cudf/round.hpp | 7 ++- 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/cpp/include/cudf/fixed_point/fixed_point.hpp b/cpp/include/cudf/fixed_point/fixed_point.hpp index a8a681f181e..09970840707 100644 --- a/cpp/include/cudf/fixed_point/fixed_point.hpp +++ b/cpp/include/cudf/fixed_point/fixed_point.hpp @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -82,39 +83,60 @@ constexpr inline auto is_supported_construction_value_type() // Helper functions for `fixed_point` type namespace detail { + /** - * @brief A function for integer exponentiation by squaring + * @brief Recursively computes integer exponentiation * - * https://simple.wikipedia.org/wiki/Exponentiation_by_squaring
- * Note: this is the iterative equivalent of the recursive definition (faster)
- * Quick-bench: http://quick-bench.com/Wg7o7HYQC9FW5M0CO0wQAjSwP_Y + * Note: This is intended to be run at compile time + * + * @tparam Rep Representation type for return type + * @tparam Base The base to be exponentiated + * @param exp The exponent to be used for exponentiation + * @return Result of `Base` to the power of `exponent` of type `Rep` + */ +template +CUDF_HOST_DEVICE inline constexpr Rep get_power(int32_t exp) +{ + return (exp > 0) ? Rep(Base) * get_power(exp - 1) : 1; +} + +/** + * @brief Implementation of integer exponentiation by array lookup * * @tparam Rep Representation type for return type * @tparam Base The base to be exponentiated + * @tparam Exponents The exponents for the array entries * @param exponent The exponent to be used for exponentiation * @return Result of `Base` to the power of `exponent` of type `Rep` */ -template +CUDF_HOST_DEVICE inline Rep ipow_impl(int32_t exponent, cuda::std::index_sequence) +{ + static constexpr Rep powers[] = { get_power(Exponents)... }; + return powers[exponent]; +} + +/** + * @brief A function for integer exponentiation by array lookup + * + * @tparam Rep Representation type for return type + * @tparam Base The base to be exponentiated + * @param exponent The exponent to be used for exponentiation + * @return Result of `Base` to the power of `exponent` of type `Rep` + */ +template && is_supported_representation_type())>* = nullptr> CUDF_HOST_DEVICE inline Rep ipow(T exponent) { cudf_assert(exponent >= 0 && "integer exponentiation with negative exponent is not possible."); - if (exponent == 0) { return static_cast(1); } - - auto extra = static_cast(1); - auto square = static_cast(Base); - while (exponent > 1) { - if (exponent & 1 /* odd */) { - extra *= square; - exponent -= 1; - } - exponent /= 2; - square *= square; + if constexpr (Base == numeric::Radix::BASE_2) { + return static_cast(1) << exponent; + } else { //BASE_10 + static constexpr auto max_exp = cuda::std::numeric_limits::digits10; + static constexpr auto exponents = cuda::std::make_index_sequence{}; + return ipow_impl(Base)>(exponent, exponents); } - return square * extra; } /** @brief Function that performs a `right shift` scale "times" on the `val` diff --git a/cpp/include/cudf/round.hpp b/cpp/include/cudf/round.hpp index 030d3d42773..ee088628b94 100644 --- a/cpp/include/cudf/round.hpp +++ b/cpp/include/cudf/round.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020-2023, NVIDIA CORPORATION. + * Copyright (c) 2020-2024, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,8 +32,9 @@ namespace cudf { /** * @brief Different rounding methods for `cudf::round` * - * Info on HALF_UP rounding: https://en.wikipedia.org/wiki/Rounding#Round_half_up - * Info on HALF_EVEN rounding: https://en.wikipedia.org/wiki/Rounding#Round_half_to_even + * Info on HALF_EVEN rounding: https://en.wikipedia.org/wiki/Rounding#Rounding_half_to_even + * Info on HALF_UP rounding: https://en.wikipedia.org/wiki/Rounding#Rounding_half_away_from_zero + * Note: HALF_UP means up in MAGNITUDE: Away from zero! Because of how Java and python define it */ enum class rounding_method : int32_t { HALF_UP, HALF_EVEN }; From 5b9f82cc78d569ff1356e785b5250b5671bd2caa Mon Sep 17 00:00:00 2001 From: Paul Mattione Date: Wed, 21 Feb 2024 14:03:44 -0500 Subject: [PATCH 2/5] Fix strings convert text, add some comments. --- cpp/include/cudf/fixed_point/fixed_point.hpp | 7 ++++++- cpp/tests/strings/fixed_point_tests.cpp | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/cpp/include/cudf/fixed_point/fixed_point.hpp b/cpp/include/cudf/fixed_point/fixed_point.hpp index 09970840707..754c5cf55c9 100644 --- a/cpp/include/cudf/fixed_point/fixed_point.hpp +++ b/cpp/include/cudf/fixed_point/fixed_point.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020-2023, NVIDIA CORPORATION. + * Copyright (c) 2020-2024, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -97,6 +97,7 @@ namespace detail { template CUDF_HOST_DEVICE inline constexpr Rep get_power(int32_t exp) { + //Compute power recursively return (exp > 0) ? Rep(Base) * get_power(exp - 1) : 1; } @@ -112,6 +113,7 @@ CUDF_HOST_DEVICE inline constexpr Rep get_power(int32_t exp) template CUDF_HOST_DEVICE inline Rep ipow_impl(int32_t exponent, cuda::std::index_sequence) { + //Compute powers at compile time, storing into array static constexpr Rep powers[] = { get_power(Exponents)... }; return powers[exponent]; } @@ -133,8 +135,11 @@ CUDF_HOST_DEVICE inline Rep ipow(T exponent) if constexpr (Base == numeric::Radix::BASE_2) { return static_cast(1) << exponent; } else { //BASE_10 + //Build index sequence for building power array at compile time static constexpr auto max_exp = cuda::std::numeric_limits::digits10; static constexpr auto exponents = cuda::std::make_index_sequence{}; + + //Get compile-time result return ipow_impl(Base)>(exponent, exponents); } } diff --git a/cpp/tests/strings/fixed_point_tests.cpp b/cpp/tests/strings/fixed_point_tests.cpp index 0a1c004d0a0..fd6224f16ec 100644 --- a/cpp/tests/strings/fixed_point_tests.cpp +++ b/cpp/tests/strings/fixed_point_tests.cpp @@ -324,7 +324,8 @@ TEST_F(StringsConvertTest, DISABLED_FixedPointStringConversionOperator) { auto const max = cuda::std::numeric_limits<__int128_t>::max(); - auto const x = numeric::decimal128{max, numeric::scale_type{-10}}; + //Must use scaled_integer, else shift (multiply) is undefined behavior (integer overflow) + auto const x = numeric::decimal128(numeric::scaled_integer{max, numeric::scale_type{-10}}); EXPECT_EQ(static_cast(x), "17014118346046923173168730371.5884105727"); auto const y = numeric::decimal128{max, numeric::scale_type{10}}; From 62f19b4c1c722310c9243183c6f5d1757f25d8a4 Mon Sep 17 00:00:00 2001 From: Paul Mattione Date: Wed, 21 Feb 2024 16:56:03 -0500 Subject: [PATCH 3/5] Fix style --- cpp/include/cudf/fixed_point/fixed_point.hpp | 20 +++++++++++--------- cpp/tests/strings/fixed_point_tests.cpp | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/cpp/include/cudf/fixed_point/fixed_point.hpp b/cpp/include/cudf/fixed_point/fixed_point.hpp index 754c5cf55c9..0e1d0da304a 100644 --- a/cpp/include/cudf/fixed_point/fixed_point.hpp +++ b/cpp/include/cudf/fixed_point/fixed_point.hpp @@ -88,7 +88,7 @@ namespace detail { * @brief Recursively computes integer exponentiation * * Note: This is intended to be run at compile time - * + * * @tparam Rep Representation type for return type * @tparam Base The base to be exponentiated * @param exp The exponent to be used for exponentiation @@ -97,7 +97,7 @@ namespace detail { template CUDF_HOST_DEVICE inline constexpr Rep get_power(int32_t exp) { - //Compute power recursively + // Compute power recursively return (exp > 0) ? Rep(Base) * get_power(exp - 1) : 1; } @@ -113,8 +113,8 @@ CUDF_HOST_DEVICE inline constexpr Rep get_power(int32_t exp) template CUDF_HOST_DEVICE inline Rep ipow_impl(int32_t exponent, cuda::std::index_sequence) { - //Compute powers at compile time, storing into array - static constexpr Rep powers[] = { get_power(Exponents)... }; + // Compute powers at compile time, storing into array + static constexpr Rep powers[] = {get_power(Exponents)...}; return powers[exponent]; } @@ -126,7 +126,9 @@ CUDF_HOST_DEVICE inline Rep ipow_impl(int32_t exponent, cuda::std::index_sequenc * @param exponent The exponent to be used for exponentiation * @return Result of `Base` to the power of `exponent` of type `Rep` */ -template && is_supported_representation_type())>* = nullptr> CUDF_HOST_DEVICE inline Rep ipow(T exponent) @@ -134,12 +136,12 @@ CUDF_HOST_DEVICE inline Rep ipow(T exponent) cudf_assert(exponent >= 0 && "integer exponentiation with negative exponent is not possible."); if constexpr (Base == numeric::Radix::BASE_2) { return static_cast(1) << exponent; - } else { //BASE_10 - //Build index sequence for building power array at compile time - static constexpr auto max_exp = cuda::std::numeric_limits::digits10; + } else { // BASE_10 + // Build index sequence for building power array at compile time + static constexpr auto max_exp = cuda::std::numeric_limits::digits10; static constexpr auto exponents = cuda::std::make_index_sequence{}; - //Get compile-time result + // Get compile-time result return ipow_impl(Base)>(exponent, exponents); } } diff --git a/cpp/tests/strings/fixed_point_tests.cpp b/cpp/tests/strings/fixed_point_tests.cpp index fd6224f16ec..1515d0b78fb 100644 --- a/cpp/tests/strings/fixed_point_tests.cpp +++ b/cpp/tests/strings/fixed_point_tests.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021-2023, NVIDIA CORPORATION. + * Copyright (c) 2021-2024, NVIDIA CORPORATION. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From ba2d186f626cdeedb50dbed0b43e988a2761580f Mon Sep 17 00:00:00 2001 From: Paul Mattione Date: Wed, 21 Feb 2024 17:31:33 -0500 Subject: [PATCH 4/5] Yet another style fix --- cpp/tests/strings/fixed_point_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/tests/strings/fixed_point_tests.cpp b/cpp/tests/strings/fixed_point_tests.cpp index 1515d0b78fb..9205207cc53 100644 --- a/cpp/tests/strings/fixed_point_tests.cpp +++ b/cpp/tests/strings/fixed_point_tests.cpp @@ -324,7 +324,7 @@ TEST_F(StringsConvertTest, DISABLED_FixedPointStringConversionOperator) { auto const max = cuda::std::numeric_limits<__int128_t>::max(); - //Must use scaled_integer, else shift (multiply) is undefined behavior (integer overflow) + // Must use scaled_integer, else shift (multiply) is undefined behavior (integer overflow) auto const x = numeric::decimal128(numeric::scaled_integer{max, numeric::scale_type{-10}}); EXPECT_EQ(static_cast(x), "17014118346046923173168730371.5884105727"); From 5dccd25aa84e499ac04c3cd6900cc6da865606fb Mon Sep 17 00:00:00 2001 From: Paul Mattione <156858817+pmattione-nvidia@users.noreply.github.com> Date: Thu, 22 Feb 2024 17:36:17 -0500 Subject: [PATCH 5/5] Update cpp/include/cudf/fixed_point/fixed_point.hpp Use doxygen @note Co-authored-by: Yunsong Wang --- cpp/include/cudf/fixed_point/fixed_point.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/include/cudf/fixed_point/fixed_point.hpp b/cpp/include/cudf/fixed_point/fixed_point.hpp index 0e1d0da304a..542e2b3c5c8 100644 --- a/cpp/include/cudf/fixed_point/fixed_point.hpp +++ b/cpp/include/cudf/fixed_point/fixed_point.hpp @@ -87,7 +87,7 @@ namespace detail { /** * @brief Recursively computes integer exponentiation * - * Note: This is intended to be run at compile time + * @note This is intended to be run at compile time * * @tparam Rep Representation type for return type * @tparam Base The base to be exponentiated