edponce commented on a change in pull request #10349:
URL: https://github.com/apache/arrow/pull/10349#discussion_r705850030
##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -852,24 +853,243 @@ struct LogbChecked {
}
};
+struct RoundUtil {
+ template <typename T>
+ static constexpr enable_if_t<std::is_floating_point<T>::value, bool>
IsInteger(
+ const T val) {
+ // |frac| ~ 0.0?
+ return std::floor(val) == val;
+ }
+
+ template <typename T>
+ static constexpr enable_if_t<std::is_floating_point<T>::value, bool>
IsHalfInteger(
+ const T val) {
+ // |frac| ~ 0.5?
+ return (val - std::floor(val)) == T(0.5);
+ }
+
+ // Calculate powers of ten with arbitrary integer exponent
+ template <typename T = double>
+ static enable_if_floating_point<T> Pow10(int64_t power) {
+ static constexpr T lut[] = {1e0F, 1e1F, 1e2F, 1e3F, 1e4F, 1e5F, 1e6F,
1e7F,
+ 1e8F, 1e9F, 1e10F, 1e11F, 1e12F, 1e13F, 1e14F,
1e15F};
+ int64_t lut_size = (sizeof(lut) / sizeof(*lut));
+ int64_t abs_power = std::abs(power);
+ auto pow10 = lut[std::min(abs_power, lut_size - 1)];
+ while (abs_power-- >= lut_size) {
+ pow10 *= 1e1F;
+ }
+ return (power >= 0) ? pow10 : (1 / pow10);
+ }
+};
+
+// Specializations of rounding implementations for round kernels
+template <typename, 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::trunc(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));
+ }
+};
+
+// Specializations of kernel state for round kernels
+template <typename>
+struct RoundOptionsWrapper;
+
+template <>
+struct RoundOptionsWrapper<RoundOptions> : public OptionsWrapper<RoundOptions>
{
+ using OptionsType = RoundOptions;
+ double pow10;
+
+ explicit RoundOptionsWrapper(OptionsType options) :
OptionsWrapper(std::move(options)) {
+ // Only positive of powers of 10 are used because combining multiply and
+ // division operations produced more stable rounding than using
multiply-only.
+ // Refer to NumPy's round implementation:
+ //
https://github.com/numpy/numpy/blob/7b2f20b406d27364c812f7a81a9c901afbd3600c/numpy/core/src/multiarray/calculation.c#L589
+ pow10 = RoundUtil::Pow10(std::abs(options.ndigits));
+ }
+
+ static Result<std::unique_ptr<KernelState>> Init(KernelContext* ctx,
+ const KernelInitArgs& args)
{
+ if (auto options = static_cast<const OptionsType*>(args.options)) {
+ return
arrow::internal::make_unique<RoundOptionsWrapper<OptionsType>>(*options);
+ }
+
+ return Status::Invalid(
+ "Attempted to initialize KernelState from null FunctionOptions");
+ }
Review comment:
Ok, I tried the template variant of `OptionsWrapper` and although it
made the code cleaner, it failed to compiled for some systems, so reverted to
duplicating the `OptionsWrapper::Init` definition. I think there needs to be a
refactoring of KernelState and related parts to support validating kernel
options and extending kernel state in a simpler manner. There are different
patterns being used in the code to fulfill these. But this is a separate issue
from this PR.
--
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]
