[I] [C++][Compute] Fix correctness of decimal round_binary kernel [arrow]

["dmsa-or (via GitHub)" <gi...@apache.org>]

dmsa-or opened a new issue, #50641:
URL: https://github.com/apache/arrow/issues/50641

   ### Describe the bug, including details regarding any error messages, version, and platform.
   
   The `round_binary` kernel for decimal input does not properly compute the halfway remainder point (`half_pow10`) elementwise based on the actual `ndigits`. Instead it is always computed as if `ndigits` is 0. This leads `half_pow10` to be off by a factor of `10^(ndigits)` and throws off the rounding logic.
   
   ```python
   import pyarrow as pa
   import pyarrow.compute as pc
   from decimal import Decimal
   
   >>> pc.round_binary(Decimal("2.89"), 1)
   <pyarrow.Decimal128Scalar: Decimal('2.80')>   # should be 2.9
   >>> pc.round_binary(Decimal("-5.927"), 2)
   <pyarrow.Decimal128Scalar: Decimal('-5.920')>   # should be -5.93
   >>> pc.round_binary(Decimal("-134.2"), -1)
   <pyarrow.Decimal128Scalar: Decimal('-140.0')>   # should be -130.0
   >>> pc.round_binary(Decimal("211"), -2)
   <pyarrow.Decimal128Scalar: Decimal('300')>    # should be 200
   ```
   
   It also falls through to a no-op for any decimal input with negative scale.
   
   ```python
   negative_scale_input = pa.scalar(Decimal("3700"), type=pa.decimal128(4, -2))
   >>> pc.round_binary(negative_scale_input, -3)
   <pyarrow.Decimal128Scalar: Decimal('3.7E+3')>    # should be 4.0E+3
   ```
   
   In summary, decimal `round_binary` returns the wrong answer when `ndigits` < scale (the standard case when rounding) AND one of the following are true:
   
   - Scale < 0
   - One of the rounding modes `HALF_DOWN`, `HALF_UP`, `HALF_TOWARDS_ZERO`, `HALF_TOWARDS_INFINITY`, `HALF_TO_EVEN`, or `HALF_TO_ODD` are used,  AND:
      - `ndigits` > 0 and `input` is closer to `ceil(input, ndigits)` than `floor(input, ndigits)`, OR 
      - `ndigits` < 0, `abs(input) % 10^(-ndigits) > 0.5` (true 90% of the time for `ndigits` == -1, and even more likely for `ndigits` < -1), and `input` is closer to `ceil(input, ndigits)` than `floor(input, ndigits)`,
   
   The only cases when decimal `round_binary` gets the right answer are when:
   
   - `ndigits` == 0,
   - `ndigits` >= scale (no rounding needed),
   - scale >= 0 and either of the following are true:
      - One of the rounding modes `DOWN`, `UP`, `TOWARDS_ZERO`, or `TOWARDS_INFINITY` are used
      - `ndigits` < 0 and `abs(input) % 10^(-ndigits) <= 0.5`, which is rare
   - OR a buggy round-to-nearest mode is used and happens to return the nearest number just by luck
   
   Thus we conclude that decimal `round_binary` is completely unreliable at the moment for the most common rounding modes when `ndigits` != 0, only getting the correct answer for about 50% of inputs by chance. This is a major issue since the only reason one would want to use `round_binary` over `round` in the first place is if some elements should be rounded to nonzero `ndigits` of precision.
   
   See the current implementation: https://github.com/apache/arrow/blob/cc90e40e5703f13ef1e38dd728eda93961d336d6/cpp/src/arrow/compute/kernels/scalar_round.cc#L925
   
   --------------
   Aside: there's also a bug in the decimal implementation of HALF_TO_ODD here:
   https://github.com/apache/arrow/blob/cc90e40e5703f13ef1e38dd728eda93961d336d6/cpp/src/arrow/compute/kernels/scalar_round.cc#L394
   
   This should be `scaled += remainder.Sign() >= 0 ? 1 : -1;` similar to what the HALF_TO_EVEN mode does.
   
   Here is the effect of the bug:
   ```python
   >>> pc.round(Decimal("-52.5"), 0, "half_to_odd")
   <pyarrow.Decimal128Scalar: Decimal('-51.0')>   # should be -53.0
   ```
   
   ### Component(s)
   
   C++, Python


-- 
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: issues-unsubscribe@arrow.apache.org.apache.org

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

Re: [I] [Python][C++][Compute] Fix correctness of decimal round_binary kernel [arrow]

["pitrou (via GitHub)" <gi...@apache.org>]

pitrou closed issue #50641: [Python][C++][Compute] Fix correctness of decimal round_binary kernel
URL: https://github.com/apache/arrow/issues/50641


-- 
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: issues-unsubscribe@arrow.apache.org

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

Re: [I] [Python][C++][Compute] Fix correctness of decimal round_binary kernel [arrow]

["pitrou (via GitHub)" <gi...@apache.org>]

pitrou commented on issue #50641:
URL: https://github.com/apache/arrow/issues/50641#issuecomment-5101848142

   Issue resolved by pull request 50642
   https://github.com/apache/arrow/pull/50642


-- 
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-unsubscribe@arrow.apache.org

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