Re: [PATCH] SEQ BUG
Paul Eggert [EMAIL PROTECTED] wrote: Pádraig Brady [EMAIL PROTECTED] writes: The attached patch handles this by only counting signficant digits from the operands. I'd rather use the idea I proposed earlier. Here's an implementation of it, which works on all the test cases in your patch. In addition, it works on the wilder counterexamples I suggested (which alas we can't put into the test suite since they're machine-specific). 2007-06-22 Paul Eggert [EMAIL PROTECTED] * NEWS: seq no longer mishandles obvious cases like seq 0 0.01 0.03 by omitting the last output number. * doc/coreutils.texi (seq invocation): Remove advice about workaround for seq off-by-one problem, since the bug is fixed now. Replace it with more-generic advice about rounding error. * src/seq.c (long_double_format, print_numbers): New arg NUMERIC_FORMAT. All uses changed. 2007-06-22 Pádraig Brady [EMAIL PROTECTED] (trivial change) * tests/seq/basic: Add test cases for seq off-by-one problem. Thanks to both of you! I've applied that patch with minor changes to make the new code use the STREQ macro rather than using strcmp directly. That has been informal policy for years, since using STREQ makes the code more readable. In fact, I think it's worth an addition to Makefile.maint, so converted a few more and added a rule to enforce the policy: 2007-06-23 Jim Meyering [EMAIL PROTECTED] Prefer STREQ (a, b) over strcmp (a, b) == 0; similar for != 0. * src/base64.c (main): Likewise. * src/install.c (setdefaultfilecon): Likewise. * src/sort.c (main): Likewise. * Makefile.maint (sc_prohibit_strcmp): New rule. * .x-sc_prohibit_strcmp: New file, to list the few exceptions. * Makefile.am (EXTRA_DIST): Add .x-sc_prohibit_strcmp. ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
Pádraig Brady [EMAIL PROTECTED] writes:
The attached patch handles this by
only counting signficant digits from the operands.
I'd rather use the idea I proposed earlier. Here's an implementation
of it, which works on all the test cases in your patch. In addition,
it works on the wilder counterexamples I suggested (which alas we
can't put into the test suite since they're machine-specific).
2007-06-22 Paul Eggert [EMAIL PROTECTED]
* NEWS: seq no longer mishandles obvious cases like
seq 0 0.01 0.03 by omitting the last output number.
* doc/coreutils.texi (seq invocation): Remove advice about workaround
for seq off-by-one problem, since the bug is fixed now. Replace
it with more-generic advice about rounding error.
* src/seq.c (long_double_format, print_numbers):
New arg NUMERIC_FORMAT. All uses changed.
2007-06-22 Pádraig Brady [EMAIL PROTECTED] (trivial change)
* tests/seq/basic: Add test cases for seq off-by-one problem.
diff --git a/NEWS b/NEWS
index c587fe7..a588896 100644
--- a/NEWS
+++ b/NEWS
@@ -48,6 +48,9 @@ GNU coreutils NEWS-*-
outline -*-
ln=target attribute) would mistakenly output the string target
before the name of each symlink. [introduced in coreutils-6.0]
+ seq no longer mishandles obvious cases like seq 0 0.01 0.03,
+ so workarounds like seq 0 0.01 0.031 are no longer needed.
+
split --line-bytes=N (-C N) no longer creates an empty file
[this bug is present at least as far back as textutils-1.22 (Jan, 1997)]
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 7290ab2..42558a3 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -14052,34 +14052,16 @@ $ seq 18446744073709551616 1 18446744073709551618
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.03} printed as the last number in this example:
+Be careful when using @command{seq} with outlandish values: otherwise
+you may see surprising results, as @command{seq} uses floating point
+internally. For example, on the x86 platform, where the internal
+representation uses a 64-bit fraction, the command:
@example
-$ seq -s ' ' 0 0.01 0.03
-0.00 0.01 0.02
+seq 1 0.001 1.009
@end example
-But that doesn't happen on many systems because @command{seq} is
-implemented using binary floating point arithmetic (via the C
[EMAIL PROTECTED] double} type)---which means decimal fractions like
@code{0.01}
-cannot be represented exactly. That in turn means some nonintuitive
-conditions like @[EMAIL PROTECTED] * 3 0.03}} will end up being true.
-
-To work around that in the above example, use a slightly larger number as
-the @var{last} value:
-
[EMAIL PROTECTED]
-$ seq -s ' ' 0 0.01 0.031
-0.00 0.01 0.02 0.03
[EMAIL PROTECTED] 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.
+outputs 1.007 twice and skips 1.008.
@exitstatus
diff --git a/src/seq.c b/src/seq.c
index c59c6b5..de2a522 100644
--- a/src/seq.c
+++ b/src/seq.c
@@ -1,5 +1,5 @@
/* seq - print sequence of numbers to standard output.
- Copyright (C) 1994-2006 Free Software Foundation, Inc.
+ Copyright (C) 1994-2007 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -210,15 +210,53 @@ print_numbers (char const *fmt,
long double first, long double step, long double last)
{
long double i;
+ long double x0 IF_LINT (= 0);
for (i = 0; /* empty */; i++)
{
long double x = first + i * step;
+
if (step 0 ? x last : last x)
- break;
+ {
+ /* If we go one past the end, but that number prints the
+same way last does, and prints differently from the
+previous number, then print last. This avoids problems
+with rounding. For example, with the x86 it causes seq
+0 0.01 0.03 to print 0.03 instead of
+stopping at 0.02. */
+
+ if (i)
+ {
+ char *x_str = NULL;
+ char *last_str = NULL;
+ if (asprintf (x_str, fmt, x) 0
+ || asprintf (last_str, fmt, last) 0)
+ xalloc_die ();
+
+ if (strcmp (x_str, last_str) == 0)
+ {
+ char *x0_str = NULL;
+ if (asprintf (x0_str, fmt, x0) 0)
+
Re: [PATCH] SEQ BUG
Paul Eggert wrote: + /* If we go one past the end, but that number prints the + same way last does, and prints differently from the + previous number, then print last. This avoids problems + with rounding. For example, with the x86 it causes seq + 0 0.01 0.03 to print 0.03 instead of + stopping at 0.02. */ I haven't time to look at this now, but will soon. A couple of points came to mind. Is it OK to look at just 1 value after last? Aren't you susceptible to whatever rounding printf does internally? My approach was to pull as much info from the user supplied _strings_ which are infinite precision. cheers, Pádraig. ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
Pádraig Brady [EMAIL PROTECTED] writes: I haven't time to look at this now, but will soon. Thanks. A couple of points came to mind. Is it OK to look at just 1 value after last? It should not be necessary to look 2 values after last. The problem we're trying to address is the bug where seq doesn't print last. Looking just 1 value after last should suffice to detect the bug. Aren't you susceptible to whatever rounding printf does internally? Yes, that's quite true. One can easily construct examples where the revised seq is mathematically incorrect, due to this problem. For example, on x86: $ seq -f %.20g 0 0.1 1.3 0 0.1 0.2 0.3001 0.4001 0.5 0.6002 0.6999 0.8001 0.9003 1 1.1 1.2 This problem arises because 1.3 is internally represented as 1.2995663191310057982263970188796520233154296875, which when formatted using %.20g displays as 1.3; but 0.1 is internally represented as 0.100013552527156068805425093160010874271392822265625, when you multiply that by 13 and round you get 1.30065052130349130266040447168052196502685546875, which when formatted using %.20g displays as 1.301; so seq decides that the after value is too large and does not display it. However, I don't think it's a serious problem worth fixing, since it should be clear to the user by looking at the intermediate values that rounding errors are causing the problem. We cannot avoid rounding errors in general, unless we switch to an arbitrary-precision package. Also, for this example the original seq is buggy in the same way. If you look at the source code of the change I proposed, it's easy to prove that the revised version will never print fewer numbers than the original, so it can't possibly exhibit the bug in places where the original does not already exhibit the bug. That's a nice property. The only bug that it could possibly introduce, is printing an extra value that the user does not want printed. I can't easily think of a case where that would happen, though I suppose it's possible. ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
Paul Eggert [EMAIL PROTECTED] wrote: Pádraig Brady [EMAIL PROTECTED] writes: ... Aren't you susceptible to whatever rounding printf does internally? Yes, that's quite true. One can easily construct examples where the revised seq is mathematically incorrect, due to this problem. For example, on x86: $ seq -f %.20g 0 0.1 1.3 0 0.1 0.2 0.3001 0.4001 0.5 0.6002 0.6999 0.8001 0.9003 1 1.1 1.2 I was just looking at a similar example: $ ./seq --format=%.21g .02 .01 .03 2.0007e-06 Admittedly contrived, and not really an objection. ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
f making seq act as if it were counting in decimal for fractions, instead of binary floating-point, is really something we want to consider, then why don't we actually have seq represent fractions in decimal, internally? Isn't that the only real way we could possibly expect seq to do what the user expects in every case? Possibly, this should be configurable via a flag (especially since doing calculations in decimal would be significantly slower than the current system). I'd be leaning towards requiring a flag in order to do decimal, but of course, that wouldn't save us from having to deal with why didn't it print 1.9 requests; it would just replace the usual lengthy explanation with a give the ... flag to make seq do what you want. -- My 2¢, Micah J. Cowan Programmer, musician, typesetting enthusiast, gamer... http://micah.cowan.name/ ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
Paul Eggert wrote: Pádraig Brady [EMAIL PROTECTED] writes: I haven't time to look at this now, but will soon. Thanks. A couple of points came to mind. Is it OK to look at just 1 value after last? It should not be necessary to look 2 values after last. The problem we're trying to address is the bug where seq doesn't print last. Looking just 1 value after last should suffice to detect the bug. Aren't you susceptible to whatever rounding printf does internally? Yes, that's quite true. One can easily construct examples where the revised seq is mathematically incorrect, due to this problem. For example, on x86: $ seq -f %.20g 0 0.1 1.3 0 0.1 0.2 0.3001 0.4001 0.5 0.6002 0.6999 0.8001 0.9003 1 1.1 1.2 This problem arises because 1.3 is internally represented as 1.2995663191310057982263970188796520233154296875, which when formatted using %.20g displays as 1.3; but 0.1 is internally represented as 0.100013552527156068805425093160010874271392822265625, when you multiply that by 13 and round you get 1.30065052130349130266040447168052196502685546875, which when formatted using %.20g displays as 1.301; so seq decides that the after value is too large and does not display it. Here is another way of triggering the same issue. Comparing the output from both patches, seq-pb prints an extra value (which is arguably more correct): $ ./seq-paul .1 .1000 .9 | tail -1 0.8001 $ ./seq-pb .1 .1000 .9 | tail -1 0.9003 However seq-pb incorrectly prints an extra value here. Operating up at the end of the range is unlikely, but still... $ ./seq-paul 922337203685477580.4 0.100 922337203685477580.5 922337203685477580.375 922337203685477580.500 $ ./seq-pb 922337203685477580.4 0.100 922337203685477580.5 922337203685477580.375 922337203685477580.500 922337203685477580.562 However, I don't think it's a serious problem worth fixing, since it should be clear to the user by looking at the intermediate values that rounding errors are causing the problem. We cannot avoid rounding errors in general, unless we switch to an arbitrary-precision package. Also, for this example the original seq is buggy in the same way. If you look at the source code of the change I proposed, it's easy to prove that the revised version will never print fewer numbers than the original, so it can't possibly exhibit the bug in places where the original does not already exhibit the bug. That's a nice property. The only bug that it could possibly introduce, is printing an extra value that the user does not want printed. I can't easily think of a case where that would happen, though I suppose it's possible. So Paul, Thank you for the explanation and fixing this up. You rock. Pádraig. ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
Micah Cowan [EMAIL PROTECTED] writes: f making seq act as if it were counting in decimal for fractions, instead of binary floating-point, is really something we want to consider, then why don't we actually have seq represent fractions in decimal, internally? Yes, that would be better. But it will take much more work to implement. We'd probably have to use GNU MP, or something like it. That is one more thing to configure, and one more thing to complicate installation and use. So there are at least some arguments against it. In the meantime it's nice if we can use floating-point as accurately as we can. ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
Pádraig Brady [EMAIL PROTECTED] writes: Thank you for the explanation and fixing this up. You're welcome. And thanks for supplying such a fun problem, (though perhaps I should qualify that by saying it was more fun than what I should have been working on, which was grading my students' work...). ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
Paul Eggert wrote: Pádraig Brady [EMAIL PROTECTED] writes: OK, how about the attached patch? Better, but it still has problems. For example, on my platform (Debian stable with GCC 4.2.0, x86) the command seq 0.0 0.1 0.9000 outputs something different from seq 0.0 0.1 0.9. The former stops at 0.8, the latter at 0.9. You're stretching :) I'll see if I can fix that, but I do think it's much better that what we have currently. In general, if we want seq's numeric behavior to be independent of the format of the operand numbers, I think we have to take a different approach. Here's an idea. Use sprintf to convert the numbers to a textual format and back, and then use the result of _that_ comparison instead of the internal floating-point comparison. For speed, do this only for the number one past the number that seq would ordinarily print now. Also, don't bother to print this extra number if it's textually identical to the previously printed number. Whaddya think? I'll have another look. Pádraig. ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
Pádraig Brady wrote:
Paul Eggert wrote:
Pádraig Brady [EMAIL PROTECTED] writes:
OK, how about the attached patch?
Better, but it still has problems. For example, on my platform
(Debian stable with GCC 4.2.0, x86) the command
seq 0.0 0.1 0.9000 outputs something different from
seq 0.0 0.1 0.9. The former stops at 0.8, the latter at 0.9.
You're stretching :)
I'll see if I can fix that,
The attached patch handles this by
only counting signficant digits from the operands.
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.0 +
+++ coreutils.pb/doc/coreutils.texi 2007-06-12 07:42:01.0 +
@@ -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.03} printed as the last number in this example:
-
[EMAIL PROTECTED]
-$ seq -s ' ' 0 0.01 0.03
-0.00 0.01 0.02
[EMAIL PROTECTED] example
-
-But that doesn't happen on many systems because @command{seq} is
-implemented using binary floating point arithmetic (via the C
[EMAIL PROTECTED] double} type)---which means decimal fractions like @code{0.01}
-cannot be represented exactly. That in turn means some nonintuitive
-conditions like @[EMAIL PROTECTED] * 3 0.03}} will end up being true.
-
-To work around that in the above example, use a slightly larger number as
-the @var{last} value:
-
[EMAIL PROTECTED]
-$ seq -s ' ' 0 0.01 0.031
-0.00 0.01 0.02 0.03
[EMAIL PROTECTED] 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.0 +
+++ coreutils.pb/src/Makefile.am 2007-06-12 06:43:29.0 +
@@ -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.0 +
+++ coreutils.pb/src/seq.c 2007-06-20 15:00:43.0 +
@@ -21,6 +21,7 @@
#include getopt.h
#include stdio.h
#include sys/types.h
+#include math.h
#include system.h
#include c-strtod.h
@@ -118,6 +119,9 @@
/* Number of digits after the decimal point, or INT_MAX if the
number can't easily be expressed as a fixed-point number. */
int precision;
+
+ /* Number of significat digits after the decimal point */
+ int significant;
};
typedef struct operand operand;
@@ -136,23 +140,37 @@
}
ret.width = strlen (arg);
- ret.precision = INT_MAX;
+ ret.significant = 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;
+ ret.significant = 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.significant = ret.precision = fraction_len;
+ const char* zeros = decimal_point + fraction_len;
+ while (*zeros-- == '0')
+ ret.significant--;
+ }
ret.width += (fraction_len == 0
? -1
: (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;
+ ret.significant -= exponent;
+ ret.significant = MIN (0, ret.significant);
+ }
}
return ret;
@@ -225,6 +243,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.significant != INT_MAX ? MAX (prec, first.significant): prec);
+ prec = (step.significant != INT_MAX ? MAX (prec, step.significant) : prec);
+ prec = (last.significant != INT_MAX ? MAX (prec, last.significant) : prec);
+ if (prec)
+{
+ long
Re: [PATCH] SEQ BUG
Pádraig Brady wrote:
Paul Eggert wrote:
Pádraig Brady [EMAIL PROTECTED] 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.0 +
+++ coreutils.pb/doc/coreutils.texi 2007-06-12 07:42:01.0 +
@@ -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.03} printed as the last number in this example:
-
[EMAIL PROTECTED]
-$ seq -s ' ' 0 0.01 0.03
-0.00 0.01 0.02
[EMAIL PROTECTED] example
-
-But that doesn't happen on many systems because @command{seq} is
-implemented using binary floating point arithmetic (via the C
[EMAIL PROTECTED] double} type)---which means decimal fractions like @code{0.01}
-cannot be represented exactly. That in turn means some nonintuitive
-conditions like @[EMAIL PROTECTED] * 3 0.03}} will end up being true.
-
-To work around that in the above example, use a slightly larger number as
-the @var{last} value:
-
[EMAIL PROTECTED]
-$ seq -s ' ' 0 0.01 0.031
-0.00 0.01 0.02 0.03
[EMAIL PROTECTED] 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.0 +
+++ coreutils.pb/src/Makefile.am 2007-06-12 06:43:29.0 +
@@ -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.0 +
+++ coreutils.pb/src/seq.c 2007-06-18 17:58:20.0 +
@@ -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:
Re: [PATCH] SEQ BUG
Pádraig Brady [EMAIL PROTECTED] writes: OK, how about the attached patch? Better, but it still has problems. For example, on my platform (Debian stable with GCC 4.2.0, x86) the command seq 0.0 0.1 0.9000 outputs something different from seq 0.0 0.1 0.9. The former stops at 0.8, the latter at 0.9. In general, if we want seq's numeric behavior to be independent of the format of the operand numbers, I think we have to take a different approach. Here's an idea. Use sprintf to convert the numbers to a textual format and back, and then use the result of _that_ comparison instead of the internal floating-point comparison. For speed, do this only for the number one past the number that seq would ordinarily print now. Also, don't bother to print this extra number if it's textually identical to the previously printed number. Whaddya think? ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
[PATCH] SEQ BUG
This patch makes `seq 0.1 0.1 0.9` output 0.1 to 0.9 inclusive, as expected.
The documentation for the previously required workaround is removed.
Note I changed the Makefile for seq to link $(SEQ_LIBM) rather than $(POW_LIB),
as $(POW_LIB) was empty. Is the configure test correct for pow()? as gcc 4.1.2
at least only uses the builtin one when the arguments to pow are positive
constants.
cheers,
Pádraig.
diff -Naur coreutils/doc/coreutils.texi coreutils.pb/doc/coreutils.texi
--- coreutils/doc/coreutils.texi 2007-06-12 07:28:45.0 +
+++ coreutils.pb/doc/coreutils.texi 2007-06-12 07:42:01.0 +
@@ -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.03} printed as the last number in this example:
-
[EMAIL PROTECTED]
-$ seq -s ' ' 0 0.01 0.03
-0.00 0.01 0.02
[EMAIL PROTECTED] example
-
-But that doesn't happen on many systems because @command{seq} is
-implemented using binary floating point arithmetic (via the C
[EMAIL PROTECTED] double} type)---which means decimal fractions like @code{0.01}
-cannot be represented exactly. That in turn means some nonintuitive
-conditions like @[EMAIL PROTECTED] * 3 0.03}} will end up being true.
-
-To work around that in the above example, use a slightly larger number as
-the @var{last} value:
-
[EMAIL PROTECTED]
-$ seq -s ' ' 0 0.01 0.031
-0.00 0.01 0.02 0.03
[EMAIL PROTECTED] 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 coreutils/src/Makefile.am coreutils.pb/src/Makefile.am
--- coreutils/src/Makefile.am 2007-06-12 07:26:01.0 +
+++ coreutils.pb/src/Makefile.am 2007-06-12 06:43:29.0 +
@@ -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 coreutils/src/seq.c coreutils.pb/src/seq.c
--- coreutils/src/seq.c 2007-06-11 10:20:57.0 +
+++ coreutils.pb/src/seq.c 2007-06-13 07:13:50.0 +
@@ -21,6 +21,7 @@
#include getopt.h
#include stdio.h
#include sys/types.h
+#include math.h
#include system.h
#include c-strtod.h
@@ -225,6 +226,22 @@
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 = MAX (first.precision, step.precision);
+ prec = MAX (prec, last.precision);
+ 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 +376,8 @@
}
}
+ last.value += get_last_adjustment (first, step, last);
+
if (format_str != NULL equal_width)
{
error (0, 0, _(\
diff -Naur coreutils/tests/seq/basic coreutils.pb/tests/seq/basic
--- coreutils/tests/seq/basic 2007-06-13 07:05:33.0 +
+++ coreutils.pb/tests/seq/basic 2007-06-13 07:02:29.0 +
@@ -49,6 +49,11 @@
['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)]}],
+
['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
___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
Paul Eggert wrote: Pádraig Brady [EMAIL PROTECTED] 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 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 which is correct, but with the proposed patch it outputs this: 922337203685477580.4 922337203685477580.5 922337203685477580.6 due to rounding errors in 'long double' arithmetic (which uses a 64 bit fraction on my platform). I don't think that is a normal use case for seq, and I was basically sacrificing accuracy at this end. The right way to address this problem is to use arbitrary-precision decimal arithmetic. By comparison, fiddling with fixed-width IEEE binary formats is problematic; it may fix some cases but it'll probably introduce other problems and it'll make it harder to explain the inevitable quirks. Agreed. I'll think a bit more, whether it's possible to handle the first case better. thanks for the review! Pádraig. ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
[PATCH] SEQ BUG
John Cowan wrote:
Pádraig Brady scripsit:
The issue and work around are documented in the info page,
but why don't we do the suggestion automatically in code
(using the precision that is automatically worked out already)?
That implies using either a fixed-point or a decimal-based floating-point
package. GNU gmp seems to be the obvious candidate.
Yes you could use gmp, but for normal uses of `seq`
you could just use appropriate comparisons?
How about the following patch, and we can also
remove the workaround info from the docs.
Pádraig.
--- seq.orig.c 2007-06-08 07:50:24.0 +
+++ seq.c 2007-06-08 09:05:23.0 +
@@ -357,6 +357,10 @@
}
}
+ /* perhaps can use nextafterl? */
+ #define PRECISION 1.0E-15
+ last.value += step.value + (step.value0?-PRECISION:PRECISION);
+
if (format_str != NULL equal_width)
{
error (0, 0, _(\
___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
Pádraig Brady wrote:
John Cowan wrote:
Pádraig Brady scripsit:
The issue and work around are documented in the info page,
but why don't we do the suggestion automatically in code
(using the precision that is automatically worked out already)?
That implies using either a fixed-point or a decimal-based floating-point
package. GNU gmp seems to be the obvious candidate.
Yes you could use gmp, but for normal uses of `seq`
you could just use appropriate comparisons?
How about the following patch, and we can also
remove the workaround info from the docs.
Pádraig.
--- seq.orig.c 2007-06-08 07:50:24.0 +
+++ seq.c 2007-06-08 09:05:23.0 +
@@ -357,6 +357,10 @@
}
}
+ /* perhaps can use nextafterl? */
+ #define PRECISION 1.0E-15
+ last.value += step.value + (step.value0?-PRECISION:PRECISION);
+
if (format_str != NULL equal_width)
{
error (0, 0, _(\
Err, the above will break for `seq 0 0.99 1.99`
How about:
--- seq.orig.c 2007-06-08 07:50:24.0 +
+++ seq.c 2007-06-08 09:34:00.0 +
@@ -357,6 +357,10 @@
}
}
+ /* perhaps can use nextafterl? */
+ #define PRECISION 1.0E-15
+ last.value += (step.value0?PRECISION:-PRECISION);
+
if (format_str != NULL equal_width)
{
error (0, 0, _(\
___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
Pádraig Brady [EMAIL PROTECTED] writes: Yes you could use gmp, but for normal uses of `seq` you could just use appropriate comparisons? How about the following patch, and we can also remove the workaround info from the docs. Pádraig. --- seq.orig.c 2007-06-08 07:50:24.0 + +++ seq.c 2007-06-08 09:05:23.0 + @@ -357,6 +357,10 @@ } } + /* perhaps can use nextafterl? */ + #define PRECISION 1.0E-15 + last.value += step.value + (step.value0?-PRECISION:PRECISION); That won't be enough, since the error adds up, so if you have a small step but a big range you can still overshoot. And if the error of your step value is negative you may print more than expected. Andreas. -- Andreas Schwab, SuSE Labs, [EMAIL PROTECTED] SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. ___ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils
Re: [PATCH] SEQ BUG
Andreas Schwab wrote:
Pádraig Brady [EMAIL PROTECTED] writes:
Yes you could use gmp, but for normal uses of `seq`
you could just use appropriate comparisons?
How about the following patch, and we can also
remove the workaround info from the docs.
Pádraig.
--- seq.orig.c 2007-06-08 07:50:24.0 +
+++ seq.c 2007-06-08 09:05:23.0 +
@@ -357,6 +357,10 @@
}
}
+ /* perhaps can use nextafterl? */
+ #define PRECISION 1.0E-15
+ last.value += step.value + (step.value0?-PRECISION:PRECISION);
That won't be enough, since the error adds up, so if you have a small
step but a big range you can still overshoot. And if the error of your
step value is negative you may print more than expected.
Sure, but for *normal uses* of seq we can do better.
I'm just throwing out patches here to illustrate the general point.
I'm too busy at the moment to polish off a patch for inclusion.
The last patch would not work well for `seq 0.1 0.1 10.9 | tail` as you
suggest.
How about the following:
--- seq.orig.c 2007-06-08 07:50:24.0 +
+++ seq.c 2007-06-08 11:23:34.0 +
@@ -21,6 +21,7 @@
#include getopt.h
#include stdio.h
#include sys/types.h
+#include math.h
#include system.h
#include c-strtod.h
@@ -357,6 +358,10 @@
}
}
+ int prec = MAX (first.precision, step.precision);
+ long double margin = powl(10,-prec)/2;
+ last.value += (step.value0?margin:-margin);
+
if (format_str != NULL equal_width)
{
error (0, 0, _(\
___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
