-
-
Notifications
You must be signed in to change notification settings - Fork 921
-
-
Notifications
You must be signed in to change notification settings - Fork 921
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Kernel.sprintf("%f",...) rounds to even on MRI but JRuby rounds away from zero #4157
Comments
Interestingly enough, both JRuby and MRI produce -1.25 for BigDecimal#to_f here. MRI rounds that to -1.2 (or 1.2 for the positive form) and we round to -1.3. MRI does the same float conversion we do on the way to formatting the value, so if you dislike that you'll have to get MRI folks involved. |
Ok actually it doesn't look like MRI does anything at all; the rounding is done by snprintf. |
This post and others like it point out that often this peculiar rounding behavior is due to inaccurate representation of certain decimals in IEEE754: http://www.perlmonks.org/?node_id=991469 However this case is not a precision issue. If the value were 1.35, however, the rounding changes for that reason:
This all boils down to sprintf behavior, but I'm having trouble finding definitive information on how/why sprintf rounds half down rather than up. |
Oh btw...the simple answer to "why doesn't MRI format the BigDecimal directly" is probably "*printf doesn't know anything about BigDecimal." I'll fix the other issue for JRuby by rounding half to even in the double-related sprintf formats. |
The logic in our double-based sprintf logic attempts to round values manually by inspecting a long-form version of the double. If we need to round, and the next digit is a five, it will round toward zero (truncate) iff that five is the last digit in the long unrounded string. This appears to have been an attempt to mimic C printf's behavior of rounding "true half" to even in the presence of inaccurately-represented IEEE754 decimals. This commit changes the pre-formatting to actually format with the specified precision, allowing NumberFormat's default HALF_EVEN logic to to the work for us. Fixes jruby#4157.
It turns out that we tried to emulate the rounding behavior using this logic in Sprintf.java: private static int round(byte[] bytes, int nDigits, int roundPos, boolean roundDown) {
int next = roundPos + 1;
if (next >= nDigits || bytes[next] < '5' ||
// MRI rounds up on nnn5nnn, but not nnn5 --
// except for when they do
(roundDown && bytes[next] == '5' && next == nDigits - 1)) {
return nDigits;
}
... This was obviously an attempt to guess at sprintf behavior by observation, but it rounds down rather than to even. I've got a patch that tries to pre-round the float before this logic, but we might be able to fix this logic too. |
Well this is frustrating. My patch seems to work, but then a test in the jruby suite failed: # JRUBY-4802
def test_sprintf_float
assert_equal "0.00000", Kernel.sprintf("%.5f", 0.000004)
assert_equal "0.00001", Kernel.sprintf("%.5f", 0.000005)
assert_equal "0.00001", Kernel.sprintf("%.5f", 0.000006)
end The middle case fails because we round it to even, so it formats as 0.00000. MRI rounds it up because 0.000005 is represented inexactly as slightly more than 0.000005:
JRuby rounds to even because it seems like our 0.000005 does get represented exactly, or at least formatting it reveals no inexactness:
The first two values here are output I added to our sprintf logic: the first is a simple println of 0.000005 and the second is a NumberFormat-produced representation of the value. In all three cases no inexactness can be found. |
I had to revert the change from #4159 because it introduced new failures. This still needs to be fixed. |
It looks like this is fixed by bc2ba39 |
on top of ^^ ... this should be handled right in 9.2.6 (due #5580) :
|
Environment
JRuby 9.1.5.0 on 1.8.0_101 on x86-64
Expected Behavior
"%3.1f" % BigDecimal.new("-1.2499999999999999")
should return"-1.2"
.Actual Behavior
"%3.1f" % BigDecimal.new("-1.2499999999999999")
actually returns"-1.3"
.Additional Notes
The primary problem is consistency across Ruby implementations.
It would be also ok that
"%3.1f" % BigDecimal.new("-1.2499999999999999")
actually returned"-1.3"
if MRI would also return the same value, which it currently does not.One additional problem is that, apparently, a BigDecimal is first converted into a Float (and thereby looses precision) during the "%f" formatting. This is surprising, and thus violates The Principle Of Least Surprise. If a BigDecimal was supplied to Kernel.sprintf("%f",...), one of two outcomes would be expected:
The current situation silently produces data loss, which is surprising if ever detected.
The text was updated successfully, but these errors were encountered: