Project

General

Profile

Actions

Bug #3609

closed

Float Infinity comparisons in 1.9

Added by taw (Tomasz Wegrzanowski) over 13 years ago. Updated almost 13 years ago.

Status:
Closed
Assignee:
-
Target version:
ruby -v:
ruby 1.9.1p429 (2010-07-02 revision 28523) [i386-darwin9]
Backport:
[ruby-core:31470]

Description

=begin
The way <=> works on pretty much everything in Ruby
is that if a <=> b return 0, 1, or -1, it completely
determines the entire set of comparisons
a==b, a>=b, a>b, a<=b, a<b,
b<=>a, b==a, b>=a, b>a, b<=a, b<a.
(and if it doesn't, a==b/b==a will be both true or both false,
everything else will raise exception or return false/nil)

Float Infinity in 1.9 but not 1.8 seems to violate that.
Comparing it with strange things returns 1 if it's on the left,
but raises exception in every other way.

inf = 1.0/0.0
inf <=> "foo" # => 1
"foo" <=> inf # ArgumentError: comparison of String with Float failed

This interacts even more strangely with very large bignums and the
"if bignum converts to float, it equals that float" thing Ruby currently does
[ruby-core:31376].

inf=1.0/0.0
huge=10**500

Consistent either way:
inf >= huge # => true
huge <= inf # => true
inf < huge # => false
huge > inf # => false

Consistent only with mathematical interpretation
(or with "equal if converts, except for special cases
for infinities"):
inf <=> huge # => 1
huge<=> inf # => -1
huge < inf # => true
huge >= inf # => false

Consistent only with "equal if converts":
inf == huge # => true
huge == inf # => true
inf > huge # => false
inf <= huge # => true

Now I'd definitely prefer mathematical interpretation of floats,
to "equal if converts", but this just doesn't make any sense
no matter which way I look at it.
=end

Actions #1

Updated by marcandre (Marc-Andre Lafortune) over 13 years ago

  • Category set to core

=begin
I completely agree that Math::Float <=> "foo" should return nil.

The current behavior is due to r23742 which wanted to address the fact that Float::Infinity <=> BigDecimal("1.0E500") was returning 0 (I think, see rubydev:38681)

To fix Float::Infinity <=> "foo", the minimum that must be done is:

diff --git a/numeric.c b/numeric.c
index eb3d4be..daa5d6d 100644
--- a/numeric.c
+++ b/numeric.c
@@ -1038,7 +1038,7 @@ flo_cmp(VALUE x, VALUE y)
break;

    default:
  •   if (isinf(a) && (!rb_respond_to(y, rb_intern("infinite?")) ||
    
  •   if (isinf(a) && (rb_respond_to(y, rb_intern("infinite?")) &&
                       !RTEST(rb_funcall(y, rb_intern("infinite?"), 0, 0)))) {
          if (a > 0.0) return INT2FIX(1);
          return INT2FIX(-1);
    

The fact that <=> is not consistent with <, etc, is also a problem that need to be fixed. Either the special treatment should be extended to the other comparison operators, or the special treatment for infinity should be removed from <=>

I believe the special treatment should be removed altogether:

diff --git a/numeric.c b/numeric.c
index eb3d4be..a6c5360 100644
--- a/numeric.c
+++ b/numeric.c
@@ -1038,11 +1038,6 @@ flo_cmp(VALUE x, VALUE y)
break;

    default:
  •   if (isinf(a) && (!rb_respond_to(y, rb_intern("infinite?")) ||
    
  •                    !RTEST(rb_funcall(y, rb_intern("infinite?"), 0, 0)))) {
    
  •       if (a > 0.0) return INT2FIX(1);
    
  •       return INT2FIX(-1);
    
  •   }
      return rb_num_coerce_cmp(x, y, rb_intern("<=>"));
    
    }
    return rb_dbl_cmp(a, b);

I understand the intent, but the fact is that Float::INFINITY is a very big value, but since it is the float representation of a lot of big real numbers, like 10400, 1040000 or even Infinity itself, I feel that r23742 introduces many inconsistencies. For example, currently:

1.0e200 ** 2 <=> BigDecimal("1.0e99999") # => 1

=end

Actions #2

Updated by nobu (Nobuyoshi Nakada) over 13 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

=begin
This issue was solved with changeset r28751.
Tomasz, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

=end

Actions #3

Updated by nobu (Nobuyoshi Nakada) over 13 years ago

  • Status changed from Closed to Assigned
  • Assignee set to yugui (Yuki Sonoda)

=begin

=end

Actions #4

Updated by mame (Yusuke Endoh) over 13 years ago

  • Status changed from Assigned to Closed

=begin
Backported at r28788.
=end

Actions #5

Updated by marcandre (Marc-Andre Lafortune) over 13 years ago

  • Status changed from Closed to Open
  • Assignee deleted (yugui (Yuki Sonoda))

=begin
The patch fixes comparison with non numerics, but doesn't address the rest of the issues:

  • inconsistency with mathematics
  • inconsistency with other operators like <, <=, ..

Is there objection to removing the special test for infinity?

diff --git a/numeric.c b/numeric.c
index 740ef54..ed159ce 100644
--- a/numeric.c
+++ b/numeric.c
@@ -1039,15 +1039,6 @@ flo_cmp(VALUE x, VALUE y)
break;

    default:
  •   if (isinf(a) && (i = rb_check_funcall(y, rb_intern("infinite?"), 0, 0)) != Qundef) {
    
  •       if (RTEST(i)) {
    
  •           int j = rb_cmpint(i, x, y);
    
  •           j = (a > 0.0) ? (j > 0 ? 0 : +1) : (j < 0 ? 0 : -1);
    
  •           return INT2FIX(j);
    
  •       }
    
  •       if (a > 0.0) return INT2FIX(1);
    
  •       return INT2FIX(-1);
    
  •   }
      return rb_num_coerce_cmp(x, y, rb_intern("<=>"));
    
    }
    return rb_dbl_cmp(a, b);

=end

Actions #6

Updated by mame (Yusuke Endoh) over 13 years ago

  • Target version set to 2.0.0

=begin
Hi,

The patch fixes comparison with non numerics, but doesn't address the rest of the issues:

Indeed. I thought nobu aimed to fix only the obvious wrong condition.

Is there objection to removing the special test for infinity?

It looks like a design issue rather then code bug. So I change this
to 1.9.x.

I have no objection against removal of the code in trunk. Though,
I like rather extend the special test to other operators than remove.

--
Yusuke Endoh
=end

Actions #7

Updated by shyouhei (Shyouhei Urabe) over 13 years ago

  • Status changed from Open to Closed

=begin

=end

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0