edponce commented on a change in pull request #10349:
URL: https://github.com/apache/arrow/pull/10349#discussion_r703722756
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -852,24 +854,232 @@ struct LogbChecked {
}
};
+struct RoundUtil {
+ template <typename T>
+ static constexpr enable_if_t<std::is_floating_point<T>::value, bool>
IsApproxEqual(
+ const T a, const T b, const T abs_tol = kDefaultAbsoluteTolerance) {
+ return std::fabs(a - b) <= abs_tol;
+ }
+
+ template <typename T>
+ static constexpr enable_if_t<std::is_floating_point<T>::value, bool>
IsApproxInt(
+ const T val, const T abs_tol = kDefaultAbsoluteTolerance) {
+ // |frac| ~ 0.0?
+ return IsApproxEqual(val, std::round(val), abs_tol);
+ }
+
+ template <typename T>
+ static constexpr enable_if_t<std::is_floating_point<T>::value, bool>
IsApproxHalfInt(
+ const T val, const T abs_tol = kDefaultAbsoluteTolerance) {
+ // |frac| ~ 0.5?
+ return IsApproxEqual(val - std::floor(val), T(0.5), abs_tol);
+ }
+
+ template <typename T>
+ static enable_if_floating_point<T> Pow10(const int64_t power) {
+ const T lut[]{1e0F, 1e1F, 1e2F, 1e3F, 1e4F, 1e5F, 1e6F, 1e7F,
+ 1e8F, 1e9F, 1e10F, 1e11F, 1e12F, 1e13F, 1e14F, 1e15F,
+ 1e16F, 1e17F, 1e18F, 1e19F, 1e20F, 1e21F, 1e22F};
+ // Return NaN if index is out-of-range.
+ auto lut_size = (int64_t)(sizeof(lut) / sizeof(*lut));
+ return (power >= 0 && power < lut_size) ? lut[power] : std::nanf("");
Review comment:
Initially, that is what I did as to avoid division operations and
instead use only multiplies (`round(val * 10^3) * 10^-3`). But found out that
this was the culprit of the floating-point precision errors (well more than
should be). After looking at several rounding implementations (e.g.,
[Numpy](https://github.com/numpy/numpy/blob/7b2f20b406d27364c812f7a81a9c901afbd3600c/numpy/core/src/multiarray/calculation.c#L589)
and
[CPython](https://hg.python.org/cpython/file/1f1498fe50e5/Objects/floatobject.c#l924))
noticed that they do divide. So I applied the division and the results were
much less noisy (I wish I could explain this but seems that errors sort of
cancelled out better). I extracted CPython's round function and converted the
divisions into reciprocal multiplies and observed the same noisy behavior
output. For this reason, the round functions apply a multiply and divide with
the greater than 1 power of 10 factors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
