With a 'first' parameter up to 1.6, seq(1) will not print the '2': $ seq 1.6 .05 2 1.6 1.65 1.7 1.75 1.8 1.85 1.9 1.95 but, starting from 1.65, it will: $ seq 1.65 .05 2 1.65 1.7 1.75 1.8 1.85 1.9 1.95 2 GNU 'seq' always prints the 2.
Created attachment 189755 [details] Don't accumulate rounding errors As I was educated on current@, many exact fractions in base-10 aren't exact in base-2, so using the for() loop accumulates rounding errors. Work around that re-computing current value for every step.
Created attachment 189760 [details] Don't accumulate rounding errors
This patch looks like an improvement but will not fix everything. In some cases, a single addition causes an unexpected result: "seq 1.1 0.1 1.2" prints just 1.1, and the patch will not fix that. GNU seq prints both 1.1 and 1.2.
Created attachment 190550 [details] patch to /usr.bin/seq/seq.c I added some lines on top of Yuri's patch to handle the special case of missing 'last'. With this new patch: $ ./seq 1.6 .05 2 1.6 1.65 1.7 1.75 1.8 1.85 1.9 1.95 2 $ ./seq 1.65 .05 2 1.65 1.7 1.75 1.8 1.85 1.9 1.95 2 $ ./seq 1.1 0.1 1.2 1.1 1.2 $ ./seq 1.1 0.3 1.2 1.1
Created attachment 190656 [details] seq output from ubuntu Output generated in an Ubuntu system
Created attachment 190657 [details] seq output from FreeBSD current seq output generated in FreeBSD current with patches applied.
Created attachment 190658 [details] seq output generator generator of different seq executions.
With the last patch we generate the same output as in Ubuntu with seq from CoreUtils 8.25 The main difference is that we don't add irrelevant zeros to the right as in 2 vs 2.0
Instead of the final string comparison, could we change the loop limit to 'cur <= last + (step/2)'?
(In reply to Conrad Meyer from comment #9) Do you mean something like this? Index: seq.c =================================================================== --- seq.c (revisión: 329385) +++ seq.c (copia de trabajo) @@ -80,7 +80,10 @@ int equalize = 0; double first = 1.0; double last = 0.0; + double last_shown_value = 0.0; + double cur; double incr = 0.0; + double step; struct lconv *locale; char *fmt = NULL; const char *sep = "\n"; @@ -163,23 +166,19 @@ if (!valid_format(fmt)) errx(1, "invalid format string"); /* - * XXX to be bug for bug compatible with Plan 9 add a + * XXX to be bug for bug compatible with Plan 9 add a * newline if none found at the end of the format string. */ } else fmt = generate_format(first, incr, last, equalize, pad); - if (incr > 0) { - for (; first <= last; first += incr) { - printf(fmt, first); - fputs(sep, stdout); - } - } else { - for (; first >= last; first += incr) { - printf(fmt, first); - fputs(sep, stdout); - } + for (step = 1, cur = first; incr > 0 ? cur <= last + (step/2) : cur >= last; + cur = first + incr * step++) { + printf(fmt, cur); + fputs(sep, stdout); + last_shown_value = cur; } + if (term != NULL) fputs(term, stdout);
(In reply to fernando.apesteguia from comment #10) > Do you mean something like this? Oh, sorry, I meant (incr/2) not (step/2). And also for the negative incr case, of course. But otherwise, yes.
(In reply to Conrad Meyer from comment #11) OK, this is the new approach: Index: seq.c =================================================================== --- seq.c (revisión: 329385) +++ seq.c (copia de trabajo) @@ -80,7 +80,10 @@ int equalize = 0; double first = 1.0; double last = 0.0; + double last_shown_value = 0.0; + double cur; double incr = 0.0; + double step; struct lconv *locale; char *fmt = NULL; const char *sep = "\n"; @@ -163,23 +166,19 @@ if (!valid_format(fmt)) errx(1, "invalid format string"); /* - * XXX to be bug for bug compatible with Plan 9 add a + * XXX to be bug for bug compatible with Plan 9 add a * newline if none found at the end of the format string. */ } else fmt = generate_format(first, incr, last, equalize, pad); - if (incr > 0) { - for (; first <= last; first += incr) { - printf(fmt, first); - fputs(sep, stdout); - } - } else { - for (; first >= last; first += incr) { - printf(fmt, first); - fputs(sep, stdout); - } + for (step = 1, cur = first; incr > 0 ? cur <= last + (incr / 2) : cur >= last - (incr / 2); + cur = first + incr * step++) { + printf(fmt, cur); + fputs(sep, stdout); + last_shown_value = cur; } + if (term != NULL) fputs(term, stdout); The problem with this one is that in some cases, we are not stopping when we should. For example: $ seq 1.7 .7 10 1.7 2.4 3.1 3.8 4.5 5.2 5.9 6.6 7.3 8 8.7 9.4 <--- Should have stopped here 10.1 $ seq 1.07 .07 10 ... ... 9.96 <--- Should have stopped here 10.03 I attached a new test file "output_incr_over_two.txt"
Created attachment 190999 [details] seq output for new approach with cur <= last + (incr / 2)
(In reply to fernando.apesteguia from comment #12) > cur >= last - (incr / 2); This one should also be plus. > The problem with this one is that in some cases, we are not stopping when we should. Ah, the problem is that 'last' is not a multiple of 'incr'. Hmm. Maybe the terminating math should be: 'trunc(last / incr) * incr + (incr / 2)'. That is, round down 'last' to a multiple of 'incr' and then add the epsilon ('incr/2'). That is a little unwieldy for the for loop, of course, but the math can be moved out.
(In reply to Conrad Meyer from comment #14) Hm, no, that's still wrong.
(In reply to Conrad Meyer from comment #15) I tried to pre-calculate the number of steps and iterate regardless of the cur<=last condition but it doesn't work either. I can't see how we can get rid of the rounding error by playing with the numbers.
Ok, GNU seq basically takes the same approach as your comment #4 patch. So I've come around to that.
(In reply to Conrad Meyer from comment #17) Yes, that's where I got the idea from after playing (unsuccessfully) with the numbers.
A commit references this bug: Author: cem Date: Tue Feb 27 22:01:40 UTC 2018 New revision: 330086 URL: https://svnweb.freebsd.org/changeset/base/330086 Log: seq(1): Consistently include 'last' for non-integers The source of error is a rounded increment being too large and thus the loop steps slightly past 'last'. Perform a final comparison using the formatted string values (truncated precision) to determine if we still need to print the 'last' value. PR: 217149 Submitted by: Fernando Apestegu?a <fernando.apesteguia AT gmail.com>, Yuri Pankov <yuripv AT icloud.com> (earlier version) Reported by: Martijn Dekker <mcdutchie AT hotmail.com> Sponsored by: Dell EMC Isilon Changes: head/usr.bin/seq/Makefile head/usr.bin/seq/seq.c head/usr.bin/seq/tests/ head/usr.bin/seq/tests/Makefile head/usr.bin/seq/tests/seq_test.sh
A commit references this bug: Author: kevans Date: Wed Jun 27 21:03:05 UTC 2018 New revision: 335739 URL: https://svnweb.freebsd.org/changeset/base/335739 Log: MFC r330086, r333155: seq(1) improvements MFC r330086 (cem): seq(1): Consistently include 'last' for non-integers The source of error is a rounded increment being too large and thus the loop steps slightly past 'last'. Perform a final comparison using the formatted string values (truncated precision) to determine if we still need to print the 'last' value. MFC r333155: seq(1): Move long_opts up with globals PR: 217149 Changes: _U stable/11/ stable/11/usr.bin/seq/Makefile stable/11/usr.bin/seq/seq.c stable/11/usr.bin/seq/tests/
A commit references this bug: Author: bdrewery Date: Wed Jun 27 22:52:32 UTC 2018 New revision: 335751 URL: https://svnweb.freebsd.org/changeset/base/335751 Log: MFC r330090: Add 'usr.bin/seq' to tests mtree after r330086 PR: 217149 Changes: _U stable/11/ stable/11/etc/mtree/BSD.tests.dist