seq: use augmenting addition to update value

[jfinkels]

This pull request fixes a floating-point precision issue with the update in the main loop of seq. Before this commit, the behavior was

$ seq 0.1 -0.1 -0.2
0.1
0.0
-0.1

which was incorrect: -0.2 should be printed as well. This was happening
because the update in the main loop was essentially

value = start + (i * increment);

which caused value to become -0.20000000000000004. That exceeds the lower bound of -0.2, so that number was not printed. By changing the update to

value += increment;

we eliminate this issue.

[tertsdiepraam]

Very interesting! This does indeed mitigate the issue, but it's not quite solved yet. I just tried:

seq 0 0.1 10000.1

and GNU includes 10000.1 while we (including this PR) don't.

I checked out the FreeBSD implementation and they use the multiplication method with an additional check for the last value:

/*
* Did we miss the last value of the range in the loop above?
*
* We might have, so check if the printable version of the last
* computed value ('cur') and desired 'last' value are equal.  If they
* are equal after formatting truncation, but 'cur' and
* 'last_shown_value' are not equal, it means the exit condition of the
* loop held true due to a rounding error and we still need to print
* 'last'.
*/
asprintf(&cur_print, fmt, cur);
asprintf(&last_print, fmt, last);
if (strcmp(cur_print, last_print) == 0 && cur != last_shown_value) {
    fputs(last_print, stdout);
    fputs(sep, stdout);
}

I would imagine that using the multiplication method would also minimize drifting of the values, although I'm not sure how big of a problem that is.

[miDeb]

AFAIK gnu uses long doubles (80 bit floats on x86), which are not available in rust.

[jfinkels]

So maybe the FreeBSD approach can offer a bit more protection, but we may not be completely safe against this type of incompatibility.

[blyxxyz]

Arbitrary-precision decimal arithmetic might be ideal for correctness, because all the input is decimal to begin with. Maybe with this crate.

That would make it trickier to support the --format option for printf-style float formatting, but numbers could be converted to floats in that case.

[jfinkels]

I have updated this pull request to use the bigdecimal crate. However, I've introduced a regression in the test for NaN arguments. Suggestions welcome, see my line comment.

[jfinkels]

I just updated this pull request with support for the NaN error message, but @blyxxyz pointed out that there are some further use cases (namely, inf and -inf arguments) that will see a regression with the code as it is today. I'll add more test cases to cover those situations and I'll ensure that the changes I propose don't reduce the functionality of seq.

[jfinkels]

Pull request #2647 adds two missing unit test cases that witness our support of seq 0 inf and seq -inf 0. I'll include those changes in this branch in order to ensure that this pull request does not cause a regression.

[jfinkels]

Hmm... I'm not sure how to do this without having a BigDecimal implementation that also supports positive and negative infinity.

[blyxxyz]

I think changing RangeBigDecimal into (BigDecimal, BigDecimal, Option<BigDecimal>) could work. With something like this logic:

• If the start or increment is inf or -inf, abort. GNU seq supports it but it's not really useful for anything.
• If the end is inf and the increment is positive or the end is -inf and the increment is negative set the end to None and just loop endlessly.
• If the end is -inf and the increment is negative or the end is inf and the increment is positive, stop immediately without any output.

[jfinkels]

I created separate pull requests for two of the suggested changes and made all of the suggested changes except:

I think changing RangeBigDecimal into (BigDecimal, BigDecimal, Option<BigDecimal>) could work. With something like this logic: [...]

I'll try that next. Thanks.

[jfinkels]

I'm going to approach this from a different angle. I'll open a new pull request when I have something. Thanks for the feedback on this.