You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/06/25 11:22:48 UTC

[GitHub] [arrow] edponce commented on a change in pull request #10349: ARROW-12744: [C++][Compute] Add rounding kernel

edponce commented on a change in pull request #10349:
URL: https://github.com/apache/arrow/pull/10349#discussion_r658690788



##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -454,6 +456,159 @@ struct PowerChecked {
   }
 };
 
+using RoundState = internal::OptionsWrapper<RoundOptions>;
+
+struct Round {
+  template <typename T, enable_if_t<std::is_floating_point<T>::value, bool> = true>
+  static constexpr bool ApproxEqual(T x, T y, int ulp = 8) {
+    // https://en.cppreference.com/w/cpp/types/numeric_limits/epsilon
+    // The machine epsilon has to be scaled to the magnitude of the values used
+    // and multiplied by the desired precision in ULPs (units in the last place)
+    return (std::fabs(x - y) <=
+            (std::numeric_limits<T>::epsilon() * std::fabs(x + y) * ulp))
+           // unless the result is subnormal
+           || (std::fabs(x - y) < std::numeric_limits<T>::min());
+  }
+
+  template <typename T, enable_if_t<std::is_floating_point<T>::value, bool> = true>
+  static constexpr bool IsHalf(T val) {
+    // |frac| == 0.5?
+    return ApproxEqual(std::fabs(std::fmod(val, T(1))), T(0.5));
+  }
+
+  template <typename T>
+  static constexpr enable_if_floating_point<T> RoundWithMultiple(T val, T mult) {
+    return (val / mult) * mult;
+  }
+
+  template <typename T>
+  static constexpr enable_if_floating_point<T> Floor(T val) {
+    return std::floor(val);
+  }
+
+  template <typename T>
+  static constexpr enable_if_floating_point<T> Ceiling(T val) {
+    return std::ceil(val);
+  }
+
+  template <typename T>
+  static constexpr enable_if_floating_point<T> Truncate(T val) {
+    return std::trunc(val);
+  }
+
+  template <typename T>
+  static constexpr enable_if_floating_point<T> TowardsInfinity(T val) {
+    return std::signbit(val) ? std::floor(val) : std::ceil(val);
+  }
+
+  template <typename T>
+  static constexpr enable_if_floating_point<T> HalfDown(T val) {
+    return std::ceil(val - T(0.5));
+  }
+
+  template <typename T>
+  static constexpr enable_if_floating_point<T> HalfUp(T val) {
+    return std::floor(val + T(0.5));
+  }
+
+  template <typename T>
+  static enable_if_floating_point<T> HalfToEven(T val) {
+    if (IsHalf(val)) {
+      auto floor = std::floor(val);
+      // Odd + 1, Even + 0
+      return floor + (std::fmod(std::fabs(floor), T(2)) >= T(1));
+    }
+    return std::round(val);
+  }
+
+  template <typename T>
+  static enable_if_floating_point<T> HalfToOdd(T val) {
+    if (IsHalf(val)) {
+      auto floor = std::floor(val);
+      // Odd + 0, Even + 1
+      return floor + (std::fmod(std::fabs(floor), T(2)) < T(1));
+    }
+    return std::round(val);
+  }
+
+  template <typename T>
+  static constexpr enable_if_floating_point<T> Nearest(T val) {
+    return std::round(val);
+  }
+
+  template <typename T>
+  static constexpr enable_if_floating_point<T> HalfTowardsZero(T val) {
+    return std::copysign(std::ceil(std::fabs(val) - T(0.5)), val);
+  }
+
+  template <typename T, typename Arg,
+            enable_if_t<std::is_integral<Arg>::value, bool> = true>
+  static constexpr enable_if_floating_point<T> Call(KernelContext* ctx, Arg arg,
+                                                    Status* st) {
+    return Call<T, T>(ctx, T(arg), st);
+  }
+
+  template <typename T, typename Arg,
+            enable_if_t<std::is_floating_point<Arg>::value, bool> = true>
+  static enable_if_floating_point<T> Call(KernelContext* ctx, Arg arg, Status* st) {
+    const RoundOptions& options = RoundState::Get(ctx);

Review comment:
       @bkietz @lidavidm How do you pass FunctionOptions to scalar arithmetic kernels? This currently segfaults when tests are ran. I am passing [`RoundOptions::Defaults()`](https://github.com/edponce/arrow/blob/ARROW-12744-Add-rounding-kernel/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc#L1117-L1119) to [`ArithmeticFunction constructor`](https://github.com/edponce/arrow/blob/ARROW-12744-Add-rounding-kernel/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc#L911-L915), but it seems options are not set properly in `KernelContext` and `RoundState::Get(ctx)` does not works properly.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org