Project

General

Profile

Actions

Bug #16518

closed

Should we rationalize Rational's numerator automatically?

Added by st0012 (Stan Lo) almost 5 years ago. Updated about 4 years ago.

Status:
Closed
Target version:
-
[ruby-core:96942]

Description

In this Rails issue we found that using BigDecimal as a Rational's numerator can have inconsistent behaviors when the denominator gets bigger. For example:

> 1 == BigDecimal(1)
=> true
> Rational(1, 1) == Rational(BigDecimal(1), 1)
=> true
> Rational(1, 6) == Rational(BigDecimal(1), 6)
=> true
> Rational(1, 60) == Rational(BigDecimal(1), 60)
=> false

So my question is, is this behavior expected?
If it's not expected, do we have a plan to fix it?
If it is, does it make sense to manually rationalize the numerator before passing it into the Rational call, in order to get a consistent result?

Updated by Eregon (Benoit Daloze) almost 5 years ago

  • Assignee set to mrkn (Kenta Murata)

This seems due to the fact Rational(BigDecimal, Integer) does not return a Rational but a BigDecimal!

That seems highly surprising to me, any Foo() "constructor" should return a Foo, isn't it?

Also a Float as numerator correctly produces a Rational, so the bug seems to be with BigDecimal as a numerator:

> Rational(BigDecimal(1), 2).class
=> BigDecimal
> Rational(BigDecimal(1).to_f, 2).class
=> Rational

Updated by st0012 (Stan Lo) almost 5 years ago

Thanks for the quick response!

I also found that it returns BigDecimal if any of its arguments is a BigDecimal:

irb(main):005:0> Rational(BigDecimal(1), 1).class
=> BigDecimal
irb(main):006:0> Rational(BigDecimal(1), BigDecimal(1)).class
=> BigDecimal
irb(main):007:0> Rational(0, BigDecimal(1)).class
=> BigDecimal
irb(main):008:0>

Updated by elct9620 (ZhengXian Qiu) almost 5 years ago

After a quick trace of the source code, the problem is BigDecimal is a subclass of Numeric but defined in Ruby.

The nurat_conver method check for argument type and convert to Rational, but the Numeric doesn't.
Ref: https://github.com/ruby/ruby/blob/5275d8bf4c43db9f057d24a26cf33ecd69f8b345/rational.c#L2632

When it calls the f_div method, it calculates it directly without converting them to Rational.

We can simply force convert the result, but I didn't think it is the best way:

return to_rational(f_div(a1, a2));

Updated by mrkn (Kenta Murata) about 4 years ago

  • Status changed from Open to Assigned
Actions #5

Updated by Anonymous about 4 years ago

  • Status changed from Assigned to Closed

Applied in changeset git|69837229d74813c807e9d079aaee33aec2c7a4d0.


rational.c: convert a numerator to rational before calling fdiv in Kernel.Rational() (#3702)

This makes Rational(BigDecimal(1), 60) == Rational(1, 60).
[Bug #16518]

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0