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.

Sign up for GitHub

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

Jump to bottom

Time.at loses precision when using BigDecimal as argument #3616

Closed
alex88 opened this issue Jan 26, 2016 · 9 comments
Closed

Time.at loses precision when using BigDecimal as argument #3616

alex88 opened this issue Jan 26, 2016 · 9 comments
Labels
core ruby 2.0-2.2
Milestone
JRuby 9.1.0.0

Comments

@alex88
Copy link

alex88 commented Jan 26, 2016

I was debugging why logstash was losing precision with a gelf input, it ended up to be a jRuby bug (maybe).

These are the steps to reproduce the issue:

➜  ~  rbenv shell jruby-9.0.4.0
➜  ~  ruby -v
jruby 9.0.4.0 (2.2.2) 2015-11-12 b9fb7aa Java HotSpot(TM) 64-Bit Server VM 25.25-b02 on 1.8.0_25-b17 +jit [darwin-x86_64]
➜  ~  irb
irb(main):001:0> RUBY_ENGINE
=> "jruby"
irb(main):002:0> RUBY_VERSION
=> "2.2.2"
irb(main):003:0> require 'time'
=> true
irb(main):004:0> require 'bigdecimal'
=> true
irb(main):005:0> bd = BigDecimal.new('0.1453769667458E10')
=> #<BigDecimal:76a3e297,'0.1453769667458E10',13(16)>
irb(main):006:0> bd.to_f
=> 1453769667.458
irb(main):007:0> Time.at(bd).to_f
=> 1453769667.0
irb(main):008:0> Time.at(bd.to_f).to_f
=> 1453769667.458
@alex88 alex88 mentioned this issue Jan 26, 2016
Closed
@headius
Copy link
Member

headius commented Jan 26, 2016

This is a known issue...as in I've known about it. MRI represents their Time object internally with much more type flexibility...Bignums, Bigdecimals, Rationals. We do not have this flexibility wired into our various Time constructors and methods.

In this case, I'd guess we're simply truncating to Fixnum any input to Time.at.

@alex88 I wonder if you'd like to expand the test cases for Time.at (and friends) in https://github.com/ruby/spec? It appears we're failing two cases related to to_r coercion, but it looks like the available cases could be improved.

@alex88
Copy link
Author

alex88 commented Jan 26, 2016

@headius correct me if I'm wrong, but you're not truncating to Fixnum, as Time.at(bd.to_f).to_f works fine.

Anyway, I'm gonna work on ruby/spec to add the tests with BigNumber later this week

@headius
Copy link
Member

headius commented Jan 26, 2016

I have a fix for this based on a quick look at MRI's code. The two main issues were that we forced non-Float/Rational to Fixnum (rather than coercing to Rational), and our BigDecimal.to_r was broken.

@headius
Copy link
Member

headius commented Jan 26, 2016

@alex88 The incoming value for Time.at is intended to represent fractional seconds. As shown in your example, when passing a BigDecimal, we end up with an integral number of seconds from Time#to_f because we truncate to Fixnum.

@headius headius added this to the JRuby 9.1.0.0 milestone Jan 26, 2016
@alex88
Copy link
Author

alex88 commented Jan 26, 2016

Wow that was fast, thanks @headius!

@headius
Copy link
Member

headius commented Feb 14, 2016

@alex88 You're welcome! Could you add a couple quick specs to https://github.com/ruby/spec so we've got coverage?

@alex88
Copy link
Author

alex88 commented Feb 15, 2016

@headius doing that now, however jruby 9.0.4.0 have like 143 failures already, is that expected?

PS: I'm running ruby specs inside an rbenv shell jruby-9.0.4.0 if that matters

@alex88
Copy link
Author

alex88 commented Feb 15, 2016

For example the time specs:

➜  ruby-spec git:(master) ../mspec/bin/mspec core/time/at_spec.rb
jruby 9.0.4.0 (2.2.2) 2015-11-12 b9fb7aa Java HotSpot(TM) 64-Bit Server VM 25.25-b02 on 1.8.0_25-b17 +jit [darwin-x86_64]

1)
Time.at passed Numeric roundtrips a Rational produced by #to_r FAILED
Expected 2016-02-14 16:27:27 -0800
 to equal 2016-02-14 16:27:27 -0800

/Users/alessandro/Sites/ruby-spec/core/time/at_spec.rb:29:in `block in (root)'
org/jruby/RubyBasicObject.java:1633:in `instance_eval'
org/jruby/RubyEnumerable.java:1552:in `all?'
org/jruby/RubyFixnum.java:301:in `times'
org/jruby/RubyArray.java:1560:in `each'
org/jruby/RubyArray.java:1560:in `each'
/Users/alessandro/Sites/ruby-spec/core/time/at_spec.rb:3:in `<top>'
org/jruby/RubyKernel.java:957:in `load'
org/jruby/RubyBasicObject.java:1633:in `instance_eval'
org/jruby/RubyArray.java:1560:in `each'

2)
Time.at passed non-Time, non-Numeric with an argument that responds to #to_r coerces using #to_r ERROR
NoMethodError: undefined method `to_i' for #<NumericMockObject:0x1563da5 @name="rational", @null=nil>
org/jruby/RubyBasicObject.java:1592:in `method_missing'
org/jruby/RubyNumeric.java:773:in `to_int'
org/jruby/RubyTime.java:1025:in `at'
/Users/alessandro/Sites/ruby-spec/core/time/at_spec.rb:85:in `block in (root)'
org/jruby/RubyBasicObject.java:1633:in `instance_eval'
org/jruby/RubyEnumerable.java:1552:in `all?'
org/jruby/RubyFixnum.java:301:in `times'
org/jruby/RubyArray.java:1560:in `each'
org/jruby/RubyArray.java:1560:in `each'
org/jruby/RubyArray.java:1560:in `each'
/Users/alessandro/Sites/ruby-spec/core/time/at_spec.rb:3:in `<top>'
org/jruby/RubyKernel.java:957:in `load'
org/jruby/RubyBasicObject.java:1633:in `instance_eval'
org/jruby/RubyArray.java:1560:in `each'

3)
An exception occurred during: Mock.verify_count
Time.at passed non-Time, non-Numeric with an argument that responds to #to_r coerces using #to_r FAILED
Mock 'rational' expected to receive 'to_r' exactly 1 times
but received it 0 times
org/jruby/RubyArray.java:1560:in `each'
org/jruby/RubyHash.java:1343:in `each'
org/jruby/RubyBasicObject.java:1633:in `instance_eval'
org/jruby/RubyEnumerable.java:1552:in `all?'
org/jruby/RubyFixnum.java:301:in `times'
org/jruby/RubyArray.java:1560:in `each'
org/jruby/RubyArray.java:1560:in `each'
org/jruby/RubyArray.java:1560:in `each'
/Users/alessandro/Sites/ruby-spec/core/time/at_spec.rb:3:in `<top>'
org/jruby/RubyKernel.java:957:in `load'
org/jruby/RubyBasicObject.java:1633:in `instance_eval'
org/jruby/RubyArray.java:1560:in `each'

4)
Time.at with a second argument that responds to #to_r coerces using #to_r ERROR
NoMethodError: undefined method `to_i' for #<NumericMockObject:0x480bdb19 @name="rational", @null=nil>
org/jruby/RubyBasicObject.java:1592:in `method_missing'
org/jruby/RubyNumeric.java:773:in `to_int'
org/jruby/RubyTime.java:1086:in `at'
/Users/alessandro/Sites/ruby-spec/core/time/at_spec.rb:116:in `block in (root)'
org/jruby/RubyBasicObject.java:1633:in `instance_eval'
org/jruby/RubyEnumerable.java:1552:in `all?'
org/jruby/RubyFixnum.java:301:in `times'
org/jruby/RubyArray.java:1560:in `each'
org/jruby/RubyArray.java:1560:in `each'
/Users/alessandro/Sites/ruby-spec/core/time/at_spec.rb:3:in `<top>'
org/jruby/RubyKernel.java:957:in `load'
org/jruby/RubyBasicObject.java:1633:in `instance_eval'
org/jruby/RubyArray.java:1560:in `each'

5)
An exception occurred during: Mock.verify_count
Time.at with a second argument that responds to #to_r coerces using #to_r FAILED
Mock 'rational' expected to receive 'to_r' exactly 1 times
but received it 0 times
org/jruby/RubyArray.java:1560:in `each'
org/jruby/RubyHash.java:1343:in `each'
org/jruby/RubyBasicObject.java:1633:in `instance_eval'
org/jruby/RubyEnumerable.java:1552:in `all?'
org/jruby/RubyFixnum.java:301:in `times'
org/jruby/RubyArray.java:1560:in `each'
org/jruby/RubyArray.java:1560:in `each'
/Users/alessandro/Sites/ruby-spec/core/time/at_spec.rb:3:in `<top>'
org/jruby/RubyKernel.java:957:in `load'
org/jruby/RubyBasicObject.java:1633:in `instance_eval'
org/jruby/RubyArray.java:1560:in `each'
[- | ==================100%================== | 00:00:00]      3F      2E

@alex88
Copy link
Author

alex88 commented Feb 15, 2016

Just tried setting also jruby as global default implementation, same thing

@alex88 alex88 mentioned this issue Feb 15, 2016
Merged
@kares kares mentioned this issue Mar 14, 2016
Closed
@alex88 alex88 mentioned this issue Mar 15, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
core ruby 2.0-2.2
Projects
None yet
Milestone
JRuby 9.1.0.0
Development

No branches or pull requests
2 participants
@headius @alex88