Skip to content
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

Round behaves differently than MRI/JRuby #3676

Open
ntkme opened this issue Sep 28, 2024 · 8 comments
Open

Round behaves differently than MRI/JRuby #3676

ntkme opened this issue Sep 28, 2024 · 8 comments
Assignees

Comments

@ntkme
Copy link
Contributor

ntkme commented Sep 28, 2024

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
Copy link
Contributor Author

ntkme commented Sep 28, 2024

#3360 might be related

@andrykonchin
Copy link
Member

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

@eregon
Copy link
Member

eregon commented Oct 1, 2024

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
Copy link
Contributor Author

ntkme commented Oct 1, 2024

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
Copy link
Contributor Author

ntkme commented Oct 1, 2024

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
Copy link
Member

eregon commented Oct 2, 2024

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
Copy link
Contributor Author

ntkme commented Oct 2, 2024

@eregon
Copy link
Member

eregon commented Oct 3, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants