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

fix Array#sum and Enumerable#sum #4309

Merged
merged 4 commits into from Nov 17, 2016

Conversation

Projects
None yet
2 participants
@phluid61
Contributor

phluid61 commented Nov 17, 2016

This changes Array#sum and Enumerable#sum to use Kahan's compensated summation algorithm, so we have the same rounding artifacts as MRI.

It also includes some of the MRI's cascading coercion behaviour in Array#sum

Now all tests in test/mri/ruby/test_array.rb and test/mri/ruby/test_enum.rb pass, except for those depending on 'mathn' which I can't test because of an unrelated issue.

Consider this a regression fix for #4297

phluid61 added some commits Nov 16, 2016

add Kahan's compensated algorithm for Floats
Fixes part of a bug identified in #4297
change Array#sum to better match MRI
Fixes most of the tests that weren't perfect in #4297.
fix Array#sum to handle Fixnum overflow
This passes the final MRI test, referenced in #4297
@phluid61

This comment has been minimized.

Contributor

phluid61 commented Nov 17, 2016

I'm not entirely comfortable that RubyArray is now an expert on Fixnum, Bignum, Rational and Float, but the behaviour matches MRI's.

@headius

This comment has been minimized.

Member

headius commented Nov 17, 2016

Looks good! Thank you!

@headius headius merged commit 46aca6c into jruby:ruby-2.4 Nov 17, 2016

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details

@headius headius added this to the JRuby 9.2.0.0 milestone Nov 17, 2016

@phluid61 phluid61 deleted the phluid61:feature/optimise-enumerable-sum branch Nov 17, 2016

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