Re: [PATCH] SEQ BUG

[Pádraig Brady]

Pádraig Brady wrote:
> Paul Eggert wrote:
>> Pádraig Brady <address@hidden> writes:
>>
>>> This patch makes `seq 0.1 0.1 0.9` output 0.1 to 0.9 inclusive, as expected.
>> I see some problems with that patch.
>>
>> First, it continues to mishandle some similar cases.  For example,
>> 'seq 0.1 1e-1 0.9' outputs only 0.1 through 0.8, i.e. it behaves
>> differently from 'seq 0.1 0.1 0.9', which is counterintutive.  I can
>> easily see getting some bug reports about that.
> 
> Agreed. The old behaviour is better than that.
> Hmm, off the top of my head, I don't see any way to
> approximate the precision of nums in the form 1e-1

OK, how about the attached patch?

>> Second, it mishandles some cases that the current code handles
>> correctly.  For example, on my platform (Debian stable x86 with GCC
>> 4.2) 'seq 922337203685477580.4 0.1 922337203685477580.5' currently
>> outputs this:
>>
>> 922337203685477580.4
>> 922337203685477580.5

Currently seq handles this upper bound not very robustly anyway:

$ seq 922337203685477580.4 0.100 922337203685477580.5
922337203685477580.375
922337203685477580.500

cheers,
Pádraig.

diff -Naur --exclude='*.o' coreutils/doc/coreutils.texi 
coreutils.pb/doc/coreutils.texi
--- coreutils/doc/coreutils.texi        2007-06-12 07:28:45.000000000 +0000
+++ coreutils.pb/doc/coreutils.texi     2007-06-12 07:42:01.000000000 +0000
@@ -14041,35 +14041,6 @@
 18446744073709551618
 @end example
 
-Be careful when using @command{seq} with a fractional @var{increment};
-otherwise you may see surprising results.  Most people would expect to
-see @code{0.000003} printed as the last number in this example:
-
address@hidden
-$ seq -s ' ' 0 0.000001 0.000003
-0.000000 0.000001 0.000002
address@hidden example
-
-But that doesn't happen on many systems because @command{seq} is
-implemented using binary floating point arithmetic (via the C
address@hidden double} type)---which means decimal fractions like 
@code{0.000001}
-cannot be represented exactly.  That in turn means some nonintuitive
-conditions like @address@hidden * 3 > 0.000003}} will end up being true.
-
-To work around that in the above example, use a slightly larger number as
-the @var{last} value:
-
address@hidden
-$ seq -s ' ' 0 0.000001 0.0000031
-0.000000 0.000001 0.000002 0.000003
address@hidden example
-
-In general, when using an @var{increment} with a fractional part, where
-(@var{last} - @var{first}) / @var{increment} is (mathematically) a whole
-number, specify a slightly larger (or smaller, if @var{increment} is negative)
-value for @var{last} to ensure that @var{last} is the final value printed
-by seq.
-
 @exitstatus
 
 
diff -Naur --exclude='*.o' coreutils/src/Makefile.am 
coreutils.pb/src/Makefile.am
--- coreutils/src/Makefile.am   2007-06-12 07:26:01.000000000 +0000
+++ coreutils.pb/src/Makefile.am        2007-06-12 06:43:29.000000000 +0000
@@ -97,7 +97,7 @@
 printf_LDADD = $(LDADD) $(POW_LIB) $(LIBICONV)
 
 # If necessary, add -lm to resolve use of pow in lib/strtod.c.
-seq_LDADD = $(LDADD) $(POW_LIB)
+seq_LDADD = $(LDADD) $(SEQ_LIBM)
 
 # If necessary, add libraries to resolve the `pow' reference in lib/strtod.c
 # and the `nanosleep' reference in lib/xnanosleep.c.
diff -Naur --exclude='*.o' coreutils/src/seq.c coreutils.pb/src/seq.c
--- coreutils/src/seq.c 2007-06-11 10:20:57.000000000 +0000
+++ coreutils.pb/src/seq.c      2007-06-18 17:58:20.000000000 +0000
@@ -21,6 +21,7 @@
 #include <getopt.h>
 #include <stdio.h>
 #include <sys/types.h>
+#include <math.h>
 
 #include "system.h"
 #include "c-strtod.h"
@@ -138,14 +139,14 @@
   ret.width = strlen (arg);
   ret.precision = INT_MAX;
 
-  if (! arg[strcspn (arg, "eExX")] && isfinite (ret.value))
+  if (! arg[strcspn (arg, "xX")] && isfinite (ret.value))
     {
       char const *decimal_point = strchr (arg, '.');
       if (! decimal_point)
        ret.precision = 0;
       else
        {
-         size_t fraction_len = strlen (decimal_point + 1);
+         size_t fraction_len = strcspn (decimal_point+1, "eE");
          if (fraction_len <= INT_MAX)
            ret.precision = fraction_len;
          ret.width += (fraction_len == 0
@@ -153,6 +154,13 @@
                        : (decimal_point == arg
                           || ! ISDIGIT (decimal_point[-1])));
        }
+      char const *e = strchr(arg, 'e');
+      if (!e) e = strchr(arg, 'E');
+      if (e)
+       {
+         long exponent = strtol (e+1, NULL, 10);
+         ret.precision += exponent < 0 ? -exponent: 0;
+       }
     }
 
   return ret;
@@ -225,6 +233,24 @@
     fputs (terminator, stdout);
 }
 
+/* Calculate adjustment to last value so that inexactness
+   in floating point representation is not significant
+   when comparing against the last value */
+static long double
+get_last_adjustment (operand first, operand step, operand last)
+{
+  int prec=0;
+  prec = (first.precision != INT_MAX ? MAX (prec, first.precision): prec);
+  prec = (step.precision  != INT_MAX ? MAX (prec, step.precision) : prec);
+  prec = (last.precision  != INT_MAX ? MAX (prec, last.precision) : prec);
+  if (prec)
+    {
+      long double margin = powl (10, -prec)/2;
+      return step.value >= 0 ? margin: -margin;
+    }
+  return 0; /* Integers can be exactly represented, so don't adjust */
+}
+
 /* Return the default format given FIRST, STEP, and LAST.  */
 static char const *
 get_default_format (operand first, operand step, operand last)
@@ -359,6 +385,8 @@
        }
     }
 
+  last.value += get_last_adjustment (first, step, last);
+
   if (format_str != NULL && equal_width)
     {
       error (0, 0, _("\
diff -Naur --exclude='*.o' coreutils/tests/seq/basic 
coreutils.pb/tests/seq/basic
--- coreutils/tests/seq/basic   2007-06-13 07:05:33.000000000 +0000
+++ coreutils.pb/tests/seq/basic        2007-06-18 18:01:59.000000000 +0000
@@ -49,6 +49,12 @@
    ['neg-3',   qw(1 -1 0),     {OUT => [qw(1 0)]}],
    ['neg-4',   qw(1 -1 -1),    {OUT => [qw(1 0 -1)]}],
 
+   ['float-1', qw(0.8 0.1 0.9),        {OUT => [qw(0.8 0.9)]}],
+   ['float-2', qw(0.1 0.99 1.99),      {OUT => [qw(0.10 1.09)]}],
+   ['float-3', qw(10.8 0.1 10.95),     {OUT => [qw(10.8 10.9)]}],
+   ['float-4', qw(0.1 -0.1 -0.2),      {OUT => [qw(0.1 0.0 -0.1 -0.2)]}],
+   ['float-5', qw(0.8 1e-1 0.9),       {OUT => [qw(0.8 0.9)]}],
+
    ['eq-wid-1',        qw(-w 1 -1 -1), {OUT => [qw(01 00 -1)]}],
 
    # Prior to 2.0g, this test would fail on e.g., HPUX systems