brazzy / floating-point-gui.de

Website that provides concise answers to common questions about floating-point numbers.
http://floating-point-gui.de/
1.01k stars 166 forks source link

a bug in nearlyEqual? #37

Closed pearu closed 7 years ago

pearu commented 7 years ago

Hi @brazzy ,

When checking out the nearlyEqual function, I got buzzled with the following codelet:

        final float diff = Math.abs(a - b);
        ...
    } else if (a == 0 || b == 0 || diff < Float.MIN_NORMAL) {
        // a or b is zero or both are extremely close to it
        // relative error is less meaningful here
        return diff < (epsilon * Float.MIN_NORMAL);

that failed in comparing 0.3 and 0.30000000000000004 for me (I am testing the codelet in Python).

Namely, the condition diff < Float.MIN_NORMAL does not mean that a and b are "extremely close to zero", as is stated in the comment.

Take a and b that are close to eachother but both are far from zero. With such input, the above takes wrong code branch, IMHO. Or am I missing something obvious here?

I suggest the following fix:

          } else if (a == 0 || b == 0 || (diff < Float.MIN_NORMAL && absA < Float.MIN_NORMAL)) {

Best regards, Pearu

brazzy commented 7 years ago

Hello and sorry for taking so long to answer; I have been very busy lately.

You are right that the comment is wrong, and the code makes no sense there; I must have gotten confused when I wrote it. But strangely, I cannot replicate the failure in Java when I call assertTrue(nearlyEqual(0.3d, 0.30000000000000004d)); even after changing the code to use double everywhere. At that magnitude, the smallest possible difference is still larger than Float.MIN_NORMAL. I guess you used a different constant in Python?

I think the shortest way to write the condition as extended would be

} else if (a == 0 || b == 0 || (absA+absB < Float.MIN_NORMAL)) {

What do you think?

brazzy commented 7 years ago

Lost sight of this again. Still haven't been able to reproduce the error, but I've committed the fix I suggested above anyway since it is at least conceptually correct and breaks nothing else.