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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
