lidavidm commented on a change in pull request #10349:
URL: https://github.com/apache/arrow/pull/10349#discussion_r704368213
##########
File path: cpp/src/arrow/compute/api_scalar.cc
##########
@@ -175,6 +219,42 @@
ElementWiseAggregateOptions::ElementWiseAggregateOptions(bool skip_nulls)
skip_nulls(skip_nulls) {}
constexpr char ElementWiseAggregateOptions::kTypeName[];
+RoundOptions::RoundOptions(int64_t ndigits, RoundMode round_mode)
+ : FunctionOptions(internal::kRoundOptionsType),
+ ndigits(ndigits),
+ round_mode(round_mode) {
+ static_assert(RoundMode::HALF_DOWN > RoundMode::DOWN &&
+ RoundMode::HALF_DOWN > RoundMode::UP &&
+ RoundMode::HALF_DOWN > RoundMode::TOWARDS_ZERO &&
+ RoundMode::HALF_DOWN > RoundMode::TOWARDS_INFINITY &&
+ RoundMode::HALF_DOWN < RoundMode::HALF_UP &&
+ RoundMode::HALF_DOWN < RoundMode::HALF_TOWARDS_ZERO &&
+ RoundMode::HALF_DOWN < RoundMode::HALF_TOWARDS_INFINITY &&
+ RoundMode::HALF_DOWN < RoundMode::HALF_TO_EVEN &&
+ RoundMode::HALF_DOWN < RoundMode::HALF_TO_ODD,
+ "Invalid order of round modes. Modes prefixed with HALF need
to be "
+ "enumerated last with HALF_DOWN being the first among them.");
+}
+constexpr char RoundOptions::kTypeName[];
+
+MRoundOptions::MRoundOptions(double multiple, RoundMode round_mode)
+ : FunctionOptions(internal::kMRoundOptionsType),
+ multiple(multiple),
+ round_mode(round_mode) {
+ static_assert(RoundMode::HALF_DOWN > RoundMode::DOWN &&
Review comment:
nit: do we really need to have this here twice?
##########
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("");
+ }
+};
+
+// Specializations of rounding implementations for round kernels
+template <typename T, RoundMode>
+struct RoundImpl {};
+
+template <typename T>
+struct RoundImpl<T, RoundMode::DOWN> {
+ static constexpr enable_if_floating_point<T> Round(const T val) {
+ return std::floor(val);
+ }
+};
+
+template <typename T>
+struct RoundImpl<T, RoundMode::UP> {
+ static constexpr enable_if_floating_point<T> Round(const T val) {
+ return std::ceil(val);
+ }
+};
+
+template <typename T>
+struct RoundImpl<T, RoundMode::TOWARDS_ZERO> {
+ static constexpr enable_if_floating_point<T> Round(const T val) {
+ return std::signbit(val) ? std::ceil(val) : std::floor(val);
+ }
+};
+
+template <typename T>
+struct RoundImpl<T, RoundMode::TOWARDS_INFINITY> {
+ static constexpr enable_if_floating_point<T> Round(const T val) {
+ return std::signbit(val) ? std::floor(val) : std::ceil(val);
+ }
+};
+
+template <typename T>
+struct RoundImpl<T, RoundMode::HALF_DOWN> {
+ static constexpr enable_if_floating_point<T> Round(const T val) {
+ return RoundImpl<T, RoundMode::DOWN>::Round(val);
+ }
+};
+
+template <typename T>
+struct RoundImpl<T, RoundMode::HALF_UP> {
+ static constexpr enable_if_floating_point<T> Round(const T val) {
+ return RoundImpl<T, RoundMode::UP>::Round(val);
+ }
+};
+
+template <typename T>
+struct RoundImpl<T, RoundMode::HALF_TOWARDS_ZERO> {
+ static constexpr enable_if_floating_point<T> Round(const T val) {
+ return RoundImpl<T, RoundMode::TOWARDS_ZERO>::Round(val);
+ }
+};
+
+template <typename T>
+struct RoundImpl<T, RoundMode::HALF_TOWARDS_INFINITY> {
+ static constexpr enable_if_floating_point<T> Round(const T val) {
+ return RoundImpl<T, RoundMode::TOWARDS_INFINITY>::Round(val);
+ }
+};
+
+template <typename T>
+struct RoundImpl<T, RoundMode::HALF_TO_EVEN> {
+ static constexpr enable_if_floating_point<T> Round(const T val) {
+ return std::round(val * T(0.5)) * 2;
+ }
+};
+
+template <typename T>
+struct RoundImpl<T, RoundMode::HALF_TO_ODD> {
+ static constexpr enable_if_floating_point<T> Round(const T val) {
+ return std::floor(val * T(0.5)) + std::ceil(val * T(0.5));
+ }
+};
+
+template <RoundMode RndMode>
+struct Round {
+ using RoundState = OptionsWrapper<RoundOptions>;
+
+ template <typename T, typename Arg>
+ static enable_if_floating_point<Arg, T> Call(KernelContext* ctx, Arg arg,
Status* st) {
+ static_assert(std::is_same<T, Arg>::value, "");
+ static_assert(RoundMode::HALF_DOWN > RoundMode::DOWN &&
+ RoundMode::HALF_DOWN > RoundMode::UP &&
+ RoundMode::HALF_DOWN > RoundMode::TOWARDS_ZERO &&
+ RoundMode::HALF_DOWN > RoundMode::TOWARDS_INFINITY &&
+ RoundMode::HALF_DOWN < RoundMode::HALF_UP &&
+ RoundMode::HALF_DOWN < RoundMode::HALF_TOWARDS_ZERO &&
+ RoundMode::HALF_DOWN < RoundMode::HALF_TOWARDS_INFINITY
&&
+ RoundMode::HALF_DOWN < RoundMode::HALF_TO_EVEN &&
+ RoundMode::HALF_DOWN < RoundMode::HALF_TO_ODD,
+ "Round modes prefixed with HALF need to be defined last in
enum and "
+ "the first HALF entry has to be HALF_DOWN.");
+
+ auto options = RoundState::Get(ctx);
+ auto pow10 = RoundUtil::Pow10<T>(std::llabs(options.ndigits));
+ if (std::isnan(pow10)) {
+ *st = Status::Invalid("out-of-range value for rounding digits");
+ return arg;
Review comment:
It seems you'll have to use ScalarUnaryNotNullStateful and do some more
refactoring on top of that (i.e. you'd be implementing a static Exec which
validates the options and instantiates the Op, then calls out to
ScalarUnaryNotNullStateful).
##########
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:
In that case, can we reference NumPy's implementation and note that
division is more stable for us than a precomputed table?
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
