Rounding modes inconsistency between round versus sprintf
I've found this possible bug with
sprintf in our production code, but it
seems very odd that no one has reported it so I'm doubting myself.
I've tested a few versions (CRuby 2.3.1 and 2.1.9) and it was present in
To reproduce in irb:
Expected result 13 actual 12
In fact if you look at the number sequence 0.5 -> 99.5 you find the even
half will all round down and the odd half round up. 12.5.round produces the correct result (13)
If you do the same in jruby 2.2.1 you get the expected result of 13.
numeric.c: round to nearest even
- numeric.c (flo_round, int_round): support round-to-nearest-even
semantics of IEEE 754 to match sprintf behavior, and add
half:optional keyword argument for the old behavior. [Bug #12548]
numeric.c: allow nil as rounding mode option
- numeric.c (rb_num_get_rounding_option): allow nil same as the default behavior, per . [Bug #12548]
#1 [ruby-core:76260] Updated by Shyouhei Urabe 8 months ago
I doubt if this is a bug or not. The rounding mode you describe is so-called "round to the nearest even" which is described in ISO 31-0 as "generally preferable". Maybe the author of sprintf explicitly chose such mode (not sure).
It seems the problem here is whether should we specify sprintf's rounding mode or leave it implementation-defined.
#2 [ruby-core:76267] Updated by Kieran McCusker 8 months ago
Having read a bit more I see that there are indeed multiple possible ways of handling tie-breaking i.e. when a value is n.5, but is it reasonable for two implementations in ruby core (sprintf and round) to use a different method? It's at least surprising (very in my case) and will catch out anyone coming from other programming backgrounds.
#5 [ruby-core:76936] Updated by Noah Gibbs 6 months ago
It looks like floats are rounded via numeric.c, using the flo_round function. That definitely does not use round-even. Maybe it should? It already implements a rounding function, so it shouldn't be too hard to implement round-even.
It looks like sprintf uses ruby_sprintf, through a series of calls to BSD_vfprintf in vsnprintf.c. At least on my machine. And BSD_vprintf winds up calling ruby_dtoa (#defined from BSD__dtoa) with mode 3. If you look for "round-even" in util.c, you'll see an extensive comment explaining why they used round-even mode.
So basically: right now, Numeric#round uses flo_round(), while sprintf uses util.c's round-even mode.
Presumably we should change flo_round to do round-even.
#6 [ruby-core:76942] Updated by Nobuyoshi Nakada 6 months ago
I agree that round-even is preferable, but it introduces an incompatibility.
Some libraries, e.g., rexml, depend on the current behavior.
So I'm thinking an optional parameter to select rounding mode.
2.5.round #=> 3 2.5.round(:even) #=> 2
#7 [ruby-core:76953] Updated by Noah Gibbs 6 months ago
Nobu: but then, default will still be inconsistent.
Maybe add a parameter and require libraries to supply it if they care? Then later we could change the default from :even to :closest or :biased or whatever we call the common rounding behavior.
#8 [ruby-core:76956] Updated by Shyouhei Urabe 6 months ago
I doubt if we need multiple rounding modes. I guess for every situation when a specific rounding mode is mandatory, what is needed is the round-even mode. I have never experienced the needs of other modes. Almost everybody don't care what mode they use.
Defaulting to that rounding mode should just suffice. Adding parameter solves a problem that doesn't exist.
#12 [ruby-core:77001] Updated by Nobuyoshi Nakada 6 months ago
Noah Gibbs wrote:
Sounds good to me. Do we need to fix ReXML, if it depends on the current rounding mode?
The rdoc of
a string that consists of optional whitespace followed by an optional minus
sign followed by a Number followed by whitespace is converted to the IEEE 754
number that is nearest (according to the IEEE 754 round-to-nearest rule) to
the mathematical value represented by the string; any other string is
converted to NaN
#14 [ruby-core:77265] Updated by Noah Gibbs 6 months ago
Ah - looks like the ReXML docs don't need to change. The round-even mode Ruby uses is still referred to as "round to nearest, ties to even" - https://en.wikipedia.org/wiki/IEEE_floating_point#Roundings_to_nearest
#15 [ruby-core:77339] Updated by Noah Gibbs 5 months ago
- ruby -v changed from 2.3.1 to 2.4.0
- File round_even.patch added
Here's a patch that makes floating-point rounding behavior match sprintf()'s round-to-even. It also adds a unit test for the new behavior. The change doesn't break any existing unit tests.
#16 [ruby-core:77344] Updated by Nobuyoshi Nakada 5 months ago
Your patch breaks two tests.
[24/42] TestVector#test_round = 0.00 s 1) Failure: TestVector#test_round [test/matrix/test_vector.rb:162]: <Vector[1.23, 2.34, 3.4]> expected but was <Vector[1.23, 2.35, 3.4]>. [30/42] REXMLTests::FunctionsTester#test_floor_ceiling_round = 0.02 s 2) Failure: REXMLTests::FunctionsTester#test_floor_ceiling_round [test/rexml/test_functions.rb:168]: <[<b id='1'/>]> expected but was <>.
#17 Updated by Noah Gibbs 5 months ago
- File round_even.patch added
You're right. I apologize. I had been running test-ruby instead of test-all without realizing it.
It appears both tests won't work if we correctly implement the round-even behavior, so the new patch fixes those.
New patch is attached.
#22 [ruby-core:77350] Updated by Nobuyoshi Nakada 5 months ago
I have a patch too, but it breaks compatibilities in rubyspec.
1) Float#round returns rounded values for big values FAILED Expected 200000000000000000000 to have same value and type as 300000000000000000000 spec/rubyspec/core/float/round_spec.rb:70:in `block (2 levels) in <top (required)>' spec/rubyspec/core/float/round_spec.rb:3:in `<top (required)>' 2) Integer#round returns itself rounded if passed a negative value FAILED Expected 200 to have same value and type as 300 spec/rubyspec/core/integer/round_spec.rb:22:in `block (2 levels) in <top (required)>' spec/rubyspec/core/integer/round_spec.rb:4:in `<top (required)>' 3) Rational#round with no arguments (precision = 0) returns the truncated value toward the nearest integer FAILED Expected 0 to equal 1 spec/rubyspec/shared/rational/round.rb:17:in `block (3 levels) in <top (required)>' spec/rubyspec/core/rational/round_spec.rb:3:in `<top (required)>' 4) Time#strftime rounds an offset to the nearest second when formatting with %z FAILED Expected "+01:01:04" to equal "+01:01:05" spec/rubyspec/core/time/strftime_spec.rb:50:in `block (2 levels) in <top (required)>' spec/rubyspec/core/time/strftime_spec.rb:7:in `<top (required)>'
#23 [ruby-core:77356] Updated by Noah Gibbs 5 months ago
@nobu: makes sense. I'll file an issue with RubySpec to let them know that at least some of these tests aren't compatible with IEEE 754 round-nearest-to-even mode. Examining the first test failure (spec/core/float/round_spec.rb line 70), it clearly isn't since it expects 2.5 to round to 3 at that decimal place.
I'll examine the other failures and try to make sure they're all equally clear.
#26 [ruby-core:77690] Updated by Noah Gibbs 4 months ago
Benoit Daloze says that RubySpec should test previous rounding behavior for Ruby before 2.4.0, and test round-even for 2.4.0+. That makes sense to me. Opened RubySpec PR #322 (https://github.com/ruby/spec/pull/322) to reflect this.
When/if we merge Nobu's branch for this bug, we should also ask RubySpec to merge PR #322 or something very similar.
#28 [ruby-core:77961] Updated by Nobuyoshi Nakada 4 months ago
We had a discussion today, and the conclusions:
half:optional keyword argument to
- its value must be one of
:upis compatible with the existing behavior,
:evenis the new behavior compatible with
half:option isn't given or is
nil, same as
#29 Updated by Nobuyoshi Nakada 4 months ago
- Status changed from Open to Closed