0

EDIT: I'm a fool, the macro parameters should have been parenthesized too. Thanks for your quick reply! Sorry for the common error post.

I have the following main.c file

#include <stdio.h>

#define EPSILON (0.00000005f)

#define IsEqual(x,y) (    ( (x-y)*(x-y)  <= EPSILON  ) ? 1 : 0   )
//int IsEqual(float x, float y){ return  ( (x-y)*(x-y)  <= EPSILON  ) ? 1 : 0  ; }


int main(void)
{
    printf("\n10 == 3+3+4: %d\n", IsEqual(10, 3+3+4));
    printf("\n10 == (1/3.0f)*27 + 1: %d\n", IsEqual(10, (1/3.0f)*27 + 1));
    printf("\n10 == -1: %d\n", IsEqual(10, -1.0f));
    printf("\n10 == -1: %d\n", IsEqual(10, -1));
    printf("\n10 == 10: %d\n", IsEqual(10, 10));

    return 0;
}

The output of this is:

10 == 3+3+4: 0
10 == (1/3.0f)*27 + 1: 0
10 == -1: 0
10 == -1: 0
10 == 10: 1

which is clearly not the intention. But if I comment-out the function-like macro IsEqual(x,y) and instead uncomment the function int IsEqual(float x, float y), then I get the expected output:

10 == 3+3+4: 1
10 == (1/3.0f)*27 + 1: 1
10 == -1: 0
10 == -1: 0
10 == 10: 1

What is wrong with my macro definition? The compiler should implicit cast int * float to float or int - float to float, so that shouldn't be the bug. Also we can see that EPSILON isn't too small to represent as a small positive float32 because the code works correctly when I use a function rather than a fuction-like macro.

Any tips? I'm hoping to learn something about macros and/or floats today!

best ~Greg

Gregory Fenn
  • 460
  • 2
  • 13
  • You might want to change the name to 'HasEqualMagnitude', as IsEqual(-1, 1) returning true is going to confuse the next person to work on the code. – Pete Kirkham May 23 '20 at 10:45
  • (x)*(x) -(y)*(y) will not work in many circumstances because of the precision lost – 0___________ May 23 '20 at 10:47
  • 1
    Note that your function and macro will fail when the absolute value of `y` is greater than the absolute value of `x`. `printf("\n10 == 11: %d\n", IsEqual(10, 11));` prints `10 == 11: 1`, because the subtraction will result in a negative number, therefore it is `<= EPSILON`. – isrnick May 23 '20 at 11:00
  • I edited the example after @isrnick's comment to avoid confusion! – Gregory Fenn May 23 '20 at 11:10
  • This sort of “compare with a tolerance” check has very limited use. [Outside of testing and experimentation, it should generally not be used in production.](https://stackoverflow.com/a/21261885/298225) – Eric Postpischil May 23 '20 at 15:59
  • Are you sure? I've got some maths functions of the form `float f(double x)` that behave in a useful linear way when `x` is close to 0, but I have a recursive solution to compute `f` of the form `return g(f(0.5f*x))`, where `g` is cheap and easy to compute. So the algorithm repeats this recursion until `x` is very close to 0, after which a different code branch is executed. So I need a way to test for "x is approximately equal to 0". Other functions often behave specially when the float `x` is close to an int. How would you achieve this without some kind of compare-with-tolerance approach? – Gregory Fenn May 24 '20 at 16:28

3 Answers3

2

You need to put parenthesis around the arguments:

(x)*(x) - (y)*(y)

This is what your current code looks like after the preprocessor:

printf("\n10 == 3+3+4: %d\n", ( ( 10*10 - 3+3+4*3+3+4 <= (0.00000005f) ) ? 1 : 0 ));
klutt
  • 30,332
  • 17
  • 55
  • 95
0

Change this

#define IsEqual(x,y) (    ( x*x - y*y  <= EPSILON  ) ? 1 : 0   )

To

#define IsEqual(x,y) (    ( (x)*(x) - (y)*(y)  <= EPSILON  ) ? 1 : 0   )

Because Macro are expands at preprocessing time

First will expand to:

printf("\n10 == 3+3+4: %d\n", ( ( 10*10 - 3+3+4*3+3+4 <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == (1/3.0f)*27 + 1: %d\n", ( ( 10*10 - (1/3.0f)*27 + 1*(1/3.0f)*27 + 1 <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == -1: %d\n", ( ( 10*10 - -1.0f*-1.0f <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == 10: %d\n", ( ( 10*10 - 10*10 <= (0.00000005f) ) ? 1 : 0 ));

You an check this by -E with gcc will produce preprocessed source code.

Second will expand to:

printf("\n10 == 3+3+4: %d\n", ( ( (10)*(10) - (3+3+4)*(3+3+4) <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == (1/3.0f)*27 + 1: %d\n", ( ( (10)*(10) - ((1/3.0f)*27 + 1)*((1/3.0f)*27 + 1) <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == -1: %d\n", ( ( (10)*(10) - (-1.0f)*(-1.0f) <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == -1: %d\n", ( ( (10)*(10) - (-1)*(-1) <= (0.00000005f) ) ? 1 : 0 ));
printf("\n10 == 10: %d\n", ( ( (10)*(10) - (10)*(10) <= (0.00000005f) ) ? 1 : 0 ));

Thanks.

srilakshmikanthanp
  • 2,231
  • 1
  • 8
  • 25
0

IMO it should be implemented like this because if you multiply you can loose the precision and numbers not equal will become equal.

In your algorithm you need to check the sign of the parameters as well

#define EPSILON (0.00000005f)
#define isEqual(x,y)     (fabs((x) - (y)) <= EPSILON)

int main() 
{
    printf("\n10 == 3+3+4: %d\n", isEqual(10, 3+3+4));
    printf("\n10 == (1/3.0f)*27 + 1: %d\n", isEqual(10, (1/3.0f)*27 + 1));
    printf("\n10 == -1: %d\n", isEqual(10.0, -1.0f));
    printf("\n10 == -1: %d\n", isEqual(10.0, -1));
    printf("\n10 == 10: %d\n", isEqual(10, 10));
} 
0___________
  • 60,014
  • 4
  • 34
  • 74