[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 <[e-mail]> 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);