D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 6531 - assertion failure in std.range.iota
Summary: assertion failure in std.range.iota
Status: RESOLVED WORKSFORME
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: Other Windows
: P2 major
Assignee: No Owner
URL:
Keywords: bootcamp, pull
: 9877 (view as issue list)
Depends on: 7455
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-19 07:49 UTC by Andrej Mitrovic
Modified: 2020-03-21 03:56 UTC (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Andrej Mitrovic 2011-08-19 07:49:29 UTC
import std.range;
import std.stdio;

void main()
{
   auto r1 = iota(0.0, 4.0, 0.03);  // ok
   auto r2 = iota(0.0, 3.0, 0.03);  // core.exception.AssertError@std.range(4001): Assertion failure
}

I want a range in steps of 0.03, beginning at 0.0 and ending at the
closest point to 3.0.

Line 4001 is:
assert(start + count * step >= end);

There's no documentation, and no custom assert message, so I don't know what was the intention of the author.

It doesn't help that this fails in debug mode but works fine in release mode.
Comment 1 Yao Gomez 2012-02-06 12:48:21 UTC
With DMD 2.058HEAD the assertion is now at line 4136.

Doing a little bit of debugging, it seems that the line at 4135:
---
if (pastEnd < end) ++count;
---
is the culprit. With the r1 range, the count variable is correctly updated, as both pastEnd and end are different.

But with the failling range (r2), pastEnd and end have the same value!, and thus the count is not correctly updated.

Changing the previous code to:
---
if (pastEnd <= end) ++count;
---
seems to do the trick, and passes all the std.range unit tests. But I don't know if that's the correct fix.
Comment 2 Yao Gomez 2012-02-06 12:51:13 UTC
I have a patch with this solution, but I haven't pulled it yet, because I need more input in whether this is the right fix or not.
Comment 3 Martin Nowak 2012-02-06 20:27:53 UTC
The correct fix is to narrow the lhs of the comparison.
assert(cast(Value)(start + count * step) >= end)

It is a common source of bugs that the intermediate 80-bit result is used for temporaries.
Comment 4 Andrej Mitrovic 2013-04-05 10:34:42 UTC
(In reply to comment #3)
> The correct fix is to narrow the lhs of the comparison.
> assert(cast(Value)(start + count * step) >= end)

That doesn't fix the issue. 

Yao Gomez'es solution does, but I don't know if it's a proper solution.

Yao, you could try making a pull request and see if you get more comments on github. Issue 9877 might be a duplicate too.
Comment 5 Martin Nowak 2013-04-06 01:47:07 UTC
> if (pastEnd <= end) ++count;

This is definitely wrong. There are only 100 steps in [0, 0.03 ..., 3.0).
The problem was that assert(0.0 + 100 * 0.03 >= 3.0) fails because of floating-point excess precision.
Comment 6 drug007 2013-04-11 07:13:10 UTC
I propose the following:

    auto pastEnd = start + count * step;
    if (step > 0)
    {
        if (pastEnd < end) {
            ++count;
            pastEnd = start + count * step;
        }
        assert(pastEnd >= end);
    }
    else
    {
        if (pastEnd > end) {
            ++count;
            pastEnd = start + count * step;
        }
        assert(pastEnd <= end);
    }

Martin Nowak's fix is the better (simpler and more clear) but I just don't like cast.
Comment 7 Andrej Mitrovic 2013-04-11 08:13:26 UTC
(In reply to comment #6)
> Martin Nowak's fix is the better (simpler and more clear) but I just don't like
> cast.

I've tried his fix and it didn't work for me.
Comment 8 drug007 2013-04-11 08:21:21 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > Martin Nowak's fix is the better (simpler and more clear) but I just don't like
> > cast.
> 
> I've tried his fix and it didn't work for me.

Try mine. It works for me (with your example).
Comment 9 safety0ff.bugz 2013-04-11 09:34:33 UTC
Indeed, casting does not seem sufficient to force correct rounding of intermediate results.
Seems like the solution is to either assign and force rounding, or use approxEqual with appropriate constants for the error terms.
Comment 10 Martin Nowak 2013-10-30 11:15:24 UTC
*** Issue 9877 has been marked as a duplicate of this issue. ***
Comment 11 Martin Nowak 2013-10-30 11:24:42 UTC
> Indeed, casting does not seem sufficient to force correct rounding of
> intermediate results.
> Seems like the solution is to either assign and force rounding, or use
> approxEqual with appropriate constants for the error terms.

Yes, casting is optimized away by dmd.
Walter suggested to use an opaque function or inline asm to enforce rounding to
lower precision.
It seems like C99 addresses this by specifying that casts and assignments need
to be rounded to lower precision.

http://stackoverflow.com/questions/503436/how-to-deal-with-excess-precision-in-floating-point-computations/503523#503523