oracle / truffleruby

A high performance implementation of the Ruby programming language, built on GraalVM.
https://www.graalvm.org/ruby/
Other
3.03k stars 185 forks source link

Round behaves differently than MRI/JRuby #3676

Open ntkme opened 1 month ago

ntkme commented 1 month ago

MRI

irb(main):001> a = 0.8241000000000004.round(10)
=> 0.8241
irb(main):002> b = 0.8241000000000002.round(10)
=> 0.8241
irb(main):003> a == b
=> true

TruffleRuby

irb(main):001:0> a = 0.8241000000000004.round(10)
=> 0.8241
irb(main):002:0> b = 0.8241000000000002.round(10)
=> 0.8240999999999999
irb(main):003:0> a == b
=> false
ntkme commented 1 month ago

3360 might be related

andrykonchin commented 1 month ago

Thank you for the report, we'll look into it.

eregon commented 1 month ago

Something interesting about these 2 float values is they seem to be exact:

irb(main):013:0> "%.100f" % 0.8241000000000002
=> "0.8241000000000002000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
irb(main):014:0> "%.100f" % 0.8241000000000004
=> "0.8241000000000004000000000000000000000000000000000000000000000000000000000000000000000000000000000000"

So at first sight this does look like a bug in rounding. OTOH, floats are always imprecise and the process of rounding itself can lose precision (otherwise it would need very large amounts of memory & time in some edge cases).

ntkme commented 1 month ago

floats are always imprecise and the process of rounding itself can lose precision

I understand and that's why I only used "behave differently" in the title. What's interesting to me is that JRuby also runs on JVM, but it managed to behave the same way as MRI.

Also, the reason for rounding in the first place is an attempt to reduce the impact of float precision error in sass-embedded gem. In Sass language, we define "fuzzy equality" as:

Two doubles are said to be fuzzy equal to one another if either:

  • They are equal according to the compareQuietEqual predicate as defined by IEEE 754 2019, §5.11.
  • They are both finite numbers and the mathematical numbers they represent produce the same value when rounded to the nearest 1e⁻¹¹ (with ties away from zero).

This "fuzzy equality" is necessary because we have Sass implementation across Dart, JS, Ruby, C++, Java and many other languages, and different language implementations often lead to different float precision for math operations.

Specifically, Sass recently added support for CSS Color Level 4 which introduced the conversion of colors between color spaces. This involves lots of mathematic operation. What happened is that TruffleRuby generally had higher precision errors than MRI/JRuby, comparing to the Dart reference implementation. This rounding behavior difference is just the most obvious case that would occasionally fail the fuzzy equality test immediately.

ntkme commented 1 month ago

This can be worked around as:

irb(main):001:0> (0.8241000000000004 * 10**10).round.fdiv(10**10)
=> 0.8241
irb(main):002:0> (0.8241000000000002 * 10**10).round.fdiv(10**10)
=> 0.8241
eregon commented 1 month ago

We end up here at the 2nd iteration of the loop: Screenshot from 2024-10-02 11-39-58 And 1 - d2 == d.

I wonder if the handling with (d2 > 0.5) ? 1 - d2 is incorrect, because here it seems to consider they are both equally far apart but yet d < d2. We should look at what CRuby does and maybe other languages too. Maybe it's simply the last condition (return Math.abs(n) < Math.abs(n2) ? n : n2;) which is incorrect.

ntkme commented 1 month ago

I checked and found JRuby indeed ported the round implementation from CRuby, which explains why they behave the same way:

https://github.com/ruby/ruby/blob/v3_3_5/numeric.c#L2449-L2578 https://github.com/ruby/ruby/blob/v3_3_5/numeric.c#L109-L178 https://github.com/jruby/jruby/blob/9.4.8.0/core/src/main/java/org/jruby/RubyFloat.java#L1004-L1159

eregon commented 1 month ago

Right, I think we should do the same, I'm not sure why we had our own variant here. Possibly because there was a pure-Ruby implementation from Rubinius and we tried to fix it. Also that code looks good at first sight because there is no loop involved. rb_flo_round_by_rational() OTOH seems very expensive, but that's only if ndigits > 14 which is hopefully rare.