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

Some kind of integer / float bounds issue; differing behavior from historical ruby up to HEAD and jruby #2631

Closed
rickhull opened this issue Mar 16, 2022 · 7 comments
Assignees
Labels
Milestone

Comments

@rickhull
Copy link

I've been enjoying truffleruby support on GitHub Actions, turning it on and off here and there. I turned it back on and got some differing behavior:

https://github.com/rickhull/compsci/actions/runs/1995013610

  1) Failure:
CompSci::Fit::Fit.power#test_0001_must accept power data [/home/runner/work/compsci/compsci/test/fit.rb:138]:
Expected: 1.0
  Actual: NaN

Here is https://github.com/rickhull/compsci/blob/master/test/fit.rb#L127

  # y = ax^b
  describe "Fit.power" do
    it "accepts power data" do
      [0.01, 7.5, 500, 1000, 5000, 9999].each { |a|
        # [-114, -100, -10, -0.5, -0.1, 0.1, 0.75, 10, 50, 60].each { |b|
        [        -100, -10, -0.5, -0.1, 0.1, 0.75, 10, 50, 60].each { |b|
          # note: on Ruby 2.4.x and older, b == -114 throws
          # warning: Bignum out of Float range
          ary = Fit.power(@xs, @xs.map { |x| a * x**b })
          expect(ary[0]).must_be_close_to a
          expect(ary[1]).must_be_close_to b
          expect(ary[2]).must_equal 1.0
        }
      }
    end
  end

Note that the -114 line is commented out. Truffleruby passes. The failed run is when the -114 value is used. Note also the comments:

          # note: on Ruby 2.4.x and older, b == -114 throws
          # warning: Bignum out of Float range

Here is Fit.power from https://github.com/rickhull/compsci/blob/master/lib/compsci/fit.rb#L143

    ##
    # To fit a functional form: y = ax^b.
    #
    # Takes x and y values and returns [a, b, r^2].
    #
    # See: http://mathworld.wolfram.com/LeastSquaresFittingPowerLaw.html

    def self.power xs, ys
      n       = xs.size
      xys     = xs.zip(ys)
      slnxlny = sigma(xys) { |x, y| Math.log(x) * Math.log(y) }
      slnx    = sigma(xs)  { |x   | Math.log(x)               }
      slny    = sigma(ys)  { |   y| Math.log(y)               }
      slnx2   = sigma(xs)  { |x   | Math.log(x) ** 2          }

      b = (n * slnxlny - slnx * slny) / (n * slnx2 - slnx ** 2)
      a = (slny - b * slnx) / n

      return Math.exp(a), b, self.error(xys) { |x| (Math.exp(a) * (x ** b)) }
    end
@rickhull
Copy link
Author

Possibly related: #2184

@bjfish
Copy link
Contributor

bjfish commented Mar 17, 2022

@eregon I suspect we are missing some integer to float division implementation:
https://github.com/ruby/ruby/blob/0db68f023372b634603c74fca94588b457be084c/numeric.c#L3715

# TruffleRuby
% ruby -e 'p Rational(1000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146010000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146010000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146009,2000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000).to_f'
NaN
% ruby -e 'p 1000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146010000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146010000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146009.fdiv(2000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000)'
NaN

# CRuby
% ruby -e 'p Rational(1000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146010000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146010000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146009,2000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000).to_f'
500.0
% ruby -e 'p 1000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146010000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146010000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146009.fdiv(2000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000)'
500.0

@eregon
Copy link
Member

eregon commented Mar 18, 2022

Good find, yeah those numbers are huge so the usual technique of converting to double first won't work here, it'll just become inf/inf->NaN:

$ ruby -e 'p 1000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146010000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146010000000000000000000000000000000000048148248609680896326399448564623182963452541226153892315137780403285956264146009.to_f'
Infinity

The gcd stuff wouldn't help this case because Rational already ensures the gcd is 1.

I think the logic in CRuby goes here: https://github.com/ruby/ruby/blob/5ca51ddde81cd7f338d0dd495ceb8569def60647/bignum.c#L6218 (i.e., division to double of 2 bignums)

@eregon
Copy link
Member

eregon commented Mar 18, 2022

Might be helpful to check what JRuby does here.

@bjfish
Copy link
Contributor

bjfish commented Mar 18, 2022

@eregon
Copy link
Member

eregon commented Mar 18, 2022

I think we should use BigDecimal as well, that's a much easier way to get the correct answer without manually writing out a division algorithm (CRuby kind of does that).

@bjfish
Copy link
Contributor

bjfish commented Apr 5, 2022

This is now fixed at 502f2c9.

@bjfish bjfish closed this as completed Apr 5, 2022
@bjfish bjfish added this to the 22.2.0 milestone Apr 5, 2022
@eregon eregon modified the milestones: 22.2.0, 22.1.0 Apr 5, 2022
@eregon eregon added the bug label Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants