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

Loss of precision on BigDecimal operations starting from 1.7.20 #3846

Closed
cburgmer opened this issue May 4, 2016 · 3 comments
Closed

Loss of precision on BigDecimal operations starting from 1.7.20 #3846

cburgmer opened this issue May 4, 2016 · 3 comments

Comments

@cburgmer
Copy link

cburgmer commented May 4, 2016

Two examples:

$ irb
io/console not supported; tty will not be manipulated
irb(main):001:0> RUBY_DESCRIPTION
=> "jruby 1.7.19 (1.9.3p551) 2015-01-29 20786bd on Java HotSpot(TM) 64-Bit Server VM 1.8.0_51-b16 [Windows 7-amd64]"
irb(main):002:0> require 'bigdecimal'
=> true
irb(main):003:0> BigDecimal.new("1") / BigDecimal.new("3")
=> #<BigDecimal:6e6d5d29,'0.33333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333E0',200(204)>

vs

$ irb
io/console not supported; tty will not be manipulated
irb(main):001:0> RUBY_DESCRIPTION
=> "jruby 1.7.20 (1.9.3p551) 2015-05-04 3086e6a on Java HotSpot(TM) 64-Bit Server VM 1.8.0_51-b16 [Windows 7-amd64]"
irb(main):002:0> require 'bigdecimal'
=> true
irb(main):003:0> BigDecimal.new("1") / BigDecimal.new("3")
=> #<BigDecimal:13d9cbf5,'0.33333333E0',8(12)>

I would expect full precision, as previous versions of JRuby offered. The change in newer versions hasn't been flagged in the changelogs. I am assuming the change was introduced in #2537.

Considering this change is expected - unless the intention of it is made clear and an alternate solution exists, this is a blocker for us upgrading to newer JRuby versions, as it breaks existing behaviour.

@kares kares added this to the JRuby 1.7.26 milestone May 4, 2016
@kares
Copy link
Member

kares commented May 4, 2016

on 1.9.3 (which JRuby 1.7.x tries to stay compatible with) you do not get that much precision either :

1.9.3-p551 :004 > require 'bigdecimal'
 => true 
1.9.3-p551 :005 > BigDecimal.new("1") / BigDecimal.new("3")
 => #<BigDecimal:80c7d0,'0.3333333333 33333333E0',18(36)> 

... so you should probably not expect to fully revert to <= 1.7.20 behaviour

@enebo enebo modified the milestones: JRuby 1.7.26, JRuby 1.7.27 Aug 26, 2016
@headius
Copy link
Member

headius commented Mar 15, 2017

This is unlikely to be fixed in 1.7.x. It appears to still be an issue in 9k, which has a longer tail.

@enebo enebo removed this from the JRuby 1.7.27 milestone Apr 24, 2017
@kares kares self-assigned this Apr 14, 2018
kares added a commit to kares/jruby that referenced this issue Apr 15, 2018
esp. on division/multiplication - some parts now match source closer
however there's a fundamental issue in terms of MRI keeping the precision
of a constructed number, while Java's BigDecimal does not ...

so in the end we end up with different precision based on input.
this needs to be cared out carefully as going 'too far' some of the ruby
specs never finish computing (division atm) ...

closes jruby#3846, jruby#4200 (we have tests guarding against regression)
kares added a commit to kares/jruby that referenced this issue Apr 15, 2018
esp. with division/multiplication - some parts now match C source closer
however there's a fundamental difference in terms of MRI keeping the
precision of a constructed number, while Java's BigDecimal doesn't ...

thus we will tend to end up with different precisions based on input.
this needs to be cared out carefully as going 'too far', as some of the
ruby specs never finish computing (division atm) ...

closes jruby#3846, jruby#4200 (we have tests guarding against regression)
@kares kares closed this as completed in 4b004f1 Apr 15, 2018
@kares kares added this to the JRuby 9.2.0.0 milestone Apr 16, 2018
@kares
Copy link
Member

kares commented Apr 16, 2018

have put a 'fix' for this regression in for 9.2 (only), since I have made some other changes to BigDecimal.
... ATM matching up precision with MRI seems impossible without some more refactoring (time put in).
however there's some tests (such as mentioned in the report) that are expected to guard regressing ...

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

4 participants