Skip to content

Commit

Permalink
Fix string to decimal128 conversion handling large exponents (#10231)
Browse files Browse the repository at this point in the history
Closes #10221 

Fixes exponent handling for large exponents. Avoids using the `exp10` function for exponent adjusts since it only accepts and returns `double` which can become imprecise for large exponents. 

Also adds a gtest using the values from #10221.

Authors:
  - David Wendt (https://github.com/davidwendt)

Approvers:
  - Conor Hoekstra (https://github.com/codereport)
  - Paul Taylor (https://github.com/trxcllnt)

URL: #10231
  • Loading branch information
davidwendt authored Feb 9, 2022
1 parent 6e267bd commit 3fe168d
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 5 deletions.
12 changes: 8 additions & 4 deletions cpp/include/cudf/strings/detail/convert/fixed_point.cuh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, NVIDIA CORPORATION.
* Copyright (c) 2021-2022, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,6 +14,8 @@
* limitations under the License.
*/

#include <cudf/fixed_point/temporary.hpp>

#include <thrust/optional.h>
#include <thrust/pair.h>

Expand Down Expand Up @@ -150,9 +152,11 @@ __device__ DecimalType parse_decimal(char const* iter, char const* iter_end, int
exp_ten += exp_offset;

// shift the output value based on the exp_ten and the scale values
value = exp_ten < scale
? value / static_cast<UnsignedDecimalType>(exp10(static_cast<double>(scale - exp_ten)))
: value * static_cast<UnsignedDecimalType>(exp10(static_cast<double>(exp_ten - scale)));
auto const shift_adjust =
abs(scale - exp_ten) > cuda::std::numeric_limits<UnsignedDecimalType>::digits10
? cuda::std::numeric_limits<UnsignedDecimalType>::max()
: numeric::detail::exp10<UnsignedDecimalType>(abs(scale - exp_ten));
value = exp_ten < scale ? value / shift_adjust : value * shift_adjust;

return static_cast<DecimalType>(value) * (sign == 0 ? 1 : sign);
}
Expand Down
18 changes: 17 additions & 1 deletion cpp/tests/strings/fixed_point_tests.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, NVIDIA CORPORATION.
* Copyright (c) 2021-2022, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -112,6 +112,22 @@ TEST_F(StringsConvertTest, ToFixedPointDecimal128)
CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(*results, expected);
}

TEST_F(StringsConvertTest, ToFixedPointLargeScale)
{
using namespace numeric;
using RepType = cudf::device_storage_type_t<decimal128>;
using fp_wrapper = cudf::test::fixed_point_column_wrapper<RepType>;

auto const strings = cudf::test::strings_column_wrapper({"0.05", "0.06", "0.50", "5.01"});

auto const scale = scale_type{-25};
auto const type = cudf::data_type{cudf::type_to_id<decimal128>(), scale};
auto const results = cudf::strings::to_fixed_point(cudf::strings_column_view(strings), type);

auto const expected = fp_wrapper{{5, 6, 50, 501}, scale_type{-2}};
CUDF_TEST_EXPECT_COLUMNS_EQUIVALENT(*results, expected);
}

TEST_F(StringsConvertTest, FromFixedPointDecimal128)
{
using namespace numeric;
Expand Down

0 comments on commit 3fe168d

Please sign in to comment.