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.
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.
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:
|
explicit RoundBinary(const DataType& out_ty) |
Aside: there's also a bug in the decimal implementation of HALF_TO_ODD here:
|
scaled += remainder.Sign() ? 1 : -1; |
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:
>>> pc.round(Decimal("-52.5"), 0, "half_to_odd")
<pyarrow.Decimal128Scalar: Decimal('-51.0')> # should be -53.0
Component(s)
C++, Python
Describe the bug, including details regarding any error messages, version, and platform.
The
round_binarykernel for decimal input does not properly compute the halfway remainder point (half_pow10) elementwise based on the actualndigits. Instead it is always computed as ifndigitsis 0. This leadshalf_pow10to be off by a factor of10^(ndigits)and throws off the rounding logic.It also falls through to a no-op for any decimal input with negative scale.
In summary, decimal
round_binaryreturns the wrong answer whenndigits< scale (the standard case when rounding) AND one of the following are true:HALF_DOWN,HALF_UP,HALF_TOWARDS_ZERO,HALF_TOWARDS_INFINITY,HALF_TO_EVEN, orHALF_TO_ODDare used, AND:ndigits> 0 andinputis closer toceil(input, ndigits)thanfloor(input, ndigits), ORndigits< 0,abs(input) % 10^(-ndigits) > 0.5(true 90% of the time forndigits== -1, and even more likely forndigits< -1), andinputis closer toceil(input, ndigits)thanfloor(input, ndigits),The only cases when decimal
round_binarygets the right answer are when:ndigits== 0,ndigits>= scale (no rounding needed),DOWN,UP,TOWARDS_ZERO, orTOWARDS_INFINITYare usedndigits< 0 andabs(input) % 10^(-ndigits) <= 0.5, which is rareThus we conclude that decimal
round_binaryis completely unreliable at the moment for the most common rounding modes whenndigits!= 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 useround_binaryoverroundin the first place is if some elements should be rounded to nonzerondigitsof precision.See the current implementation:
arrow/cpp/src/arrow/compute/kernels/scalar_round.cc
Line 925 in cc90e40
Aside: there's also a bug in the decimal implementation of HALF_TO_ODD here:
arrow/cpp/src/arrow/compute/kernels/scalar_round.cc
Line 394 in cc90e40
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:
Component(s)
C++, Python