• via Digitalmars-d-bugs (43/43) Apr 05 2016 https://issues.dlang.org/show_bug.cgi?id=15881

via Digitalmars-d-bugs <digitalmars-d-bugs puremagic.com> writes:

https://issues.dlang.org/show_bug.cgi?id=15881

          Issue ID: 15881
           Summary: approxEqual Ignores maxAbsDiff
           Product: D
           Version: D2
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P1
         Component: phobos
          Assignee: nobody puremagic.com
          Reporter: john.michael.hall gmail.com

The current implementation of approxEqual will ignore the maxAbsDiff term in
most circumstances.

The code below compares two floating point numbers. They are sufficiently far
away that on an absolute basis the assertion should fail. However, on a
relative basis, there is less than a 1% difference between the two. 

import std.math : approxEqual, fabs;

void main()
{
    auto x = 1000.0;
    auto y = x + 10.0;
    assert(approxEqual(x, y)); //should fail

    assert(fabs((x - y) / y) <= 1E-2);
    assert(!(fabs(x - y) <= 1E-5));
    assert(!(fabs((x - y) / y) <= 1E-2 && fabs(x - y) <= 1E-5));
    assert(fabs((x - y) / y) <= 1E-2 || 1E-5 != 0 && fabs(x - y) <= 1E-5);
//this is effectively the current implementation, wrong!
}

The exact line of the approxEqual code causing the problem is 

            return fabs((lhs - rhs) / rhs) <= maxRelDiff
                || maxAbsDiff != 0 && fabs(lhs - rhs) <= maxAbsDiff;

It mirrors the last assert in the code above. The || operator short-circuits
the last part of the logical statement from being called. Perhaps changing it
to 

            return fabs((lhs - rhs) / rhs) <= maxRelDiff
                && maxAbsDiff != 0 && fabs(lhs - rhs) <= maxAbsDiff;

will resolve the issue. Alternately, 

            return (fabs((lhs - rhs) / rhs) <= maxRelDiff)
                | (maxAbsDiff != 0) && fabs(lhs - rhs) <= maxAbsDiff;

might resolve the issue.

--