From: Marc-Andre Lafortune Date: 2011-08-29T04:31:35+09:00 Subject: [ruby-core:39166] Re: [Ruby 1.9 - Bug #5227][Assigned] Float#round fails on corner cases Hi, On Sat, Aug 27, 2011 at 4:20 AM, Nobuyoshi Nakada wrote: > In fact, your patch breaks test/ruby/test_float.rb. Oh, I didn't realize that `make test-all` does not recompile modified .c files. Is there a good reason for that? In any case, I write specs in RubySpec, but ruby's tests must still be run too, of course! Indeed, I made a mistake in the inequalities for the 3 or 4 approximation (should test for binexp > 0, not < 0). Here's the differential patch: diff --git a/numeric.c b/numeric.c index 4c48355..ac38bb1 100644 --- a/numeric.c +++ b/numeric.c @@ -1517,10 +1517,10 @@ flo_round(int argc, VALUE *argv, VALUE num) So if ndigits + binexp/(3 or 4) >= 17, the result is number If ndigits + binexp/(4 or 3) < 0 the result is 0 */ - if ((long)ndigits * (4 - (binexp < 0)) + binexp < 0) { + if ((long)ndigits * (4 - (binexp > 0)) + binexp < 0) { number = 0; } - else if ((long)(ndigits - 17) * (3 + (binexp < 0)) + binexp < 0) { + else if ((long)(ndigits - 17) * (3 + (binexp > 0)) + binexp < 0) { f = pow(10, abs(ndigits)); if (ndigits < 0) { double absnum = fabs(number); The complete patch becomes: diff --git a/numeric.c b/numeric.c index ab890c6..2277296 100644 --- a/numeric.c +++ b/numeric.c @@ -1491,18 +1491,37 @@ flo_round(int argc, VALUE *argv, VALUE num) VALUE nd; double number, f; int ndigits = 0; + int binexp; long val; if (argc > 0 && rb_scan_args(argc, argv, "01", &nd) == 1) { ndigits = NUM2INT(nd); } number = RFLOAT_VALUE(num); - f = pow(10, abs(ndigits)); - - if (isinf(f)) { - if (ndigits < 0) number = 0; - } - else { + frexp (number , &binexp); + +/* Let `exp` be such that `number` is written as:"0.#{digits}e#{exp}", + i.e. such that 10 ** (exp - 1) <= |number| < 10 ** exp + Recall that up to 17 digits can be needed to represent a double, + so if ndigits + exp >= 17, the intermediate value (number * 10 ** ndigits) + will be an integer and thus the result is the original number. + If ndigits + exp <= 0, the result is 0 or "1e#{exp}", so + if ndigits + exp < 0, the result is 0. + We have: + 2 ** (binexp-1) <= |number| < 2 ** binexp + 10 ** ((binexp-1)/log_2(10)) <= |number| < 10 ** (binexp/log_2(10)) + If binexp >= 0, and since log_2(10) = 3.322259: + 10 ** (binexp/4 - 1) < |number| < 10 ** (binexp/3) + binexp/4 <= exp <= binexp/3 + If binexp <= 0, swap the /4 and the /3 + So if ndigits + binexp/(4 or 3) >= 17, the result is number + If ndigits + binexp/(3 or 4) < 0 the result is 0 +*/ + if ((long)ndigits * (4 - (binexp > 0)) + binexp < 0) { + number = 0; + } + else if ((long)(ndigits - 17) * (3 + (binexp > 0)) + binexp < 0) { + f = pow(10, abs(ndigits)); if (ndigits < 0) { double absnum = fabs(number); if (absnum < f) return INT2FIX(0);