float, double equality check: clarify functionality

[andreasbaumann]

I fail to see that ck_assert_float_eq or ck_assert_double_eq (ck_assert_ldouble_eq) are actually working. For instance:

#define _ck_assert_floating(X, OP, Y, TP, TM) do { \
  TP _ck_x = (X); \
  TP _ck_y = (Y); \
  ck_assert_msg(_ck_x OP _ck_y, \
  "Assertion '%s' failed: %s == %.*"TM"g, %s == %.*"TM"g", \
  #X" "#OP" "#Y, \
  #X, (int)CK_FLOATING_DIG, _ck_x, \
  #Y, (int)CK_FLOATING_DIG, _ck_y); \
} while (0)

#define ck_assert_float_eq(X, Y) _ck_assert_floating(X, ==, Y, float, "")

Comparing two floats this way is simply wrong, see this nice explanation:

https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/

[brarcher]

I agree with you that the utility of the floating point equality functions is extremely limited. I would expect one to only use those when the value being checked is being assigned as a constant somewhere, rather than the result of a computation.

Because floating point math is imprecise, Check provides additional macros which accept a tolerance for comparisons. These end in _tol and API details can be found here:

https://libcheck.github.io/check/doc/doxygen/html/check_8h.html

I see that the fix for the linked bug is switching to these API, which is good:

davatorium/rofi@dea962d

I'm not sure what actions should be done for Check, as the APIs are functioning as expected. One would need to know that using an exact floating point comparison would not be appropriate in most situations. Perhaps the existence of the floating point equality APIs is misleading. However, as there are at least some limited situations where they are needed, I do not think they should be deprecated. If you disagree, please let me know.

[brarcher]

One additional comment, related to the point about Check's testing.

I would expect a library to write unit tests in to get the concept of floating point comparisions correctly.

Per my description above there are several Check unit tests which verify that the floating point _eq functions work as expected. Take a look at test_ck_assert_float_with_expr for an example. Below that is the test test_ck_assert_float_eq_tol which verifies the behavior of the _tol asserts, which is likely what most users would use for floating point comparision.

[andreasbaumann]

Thanks a lot for the clarification. I can also only think of better commenting on the functions directly to
be very careful with floating point comparisions. A magical function solving all equality tests is not
possible, as it depends on the calculations done and on the range of the floating point numbers used.

I just wanted to bring the problem to attention, because I start to think, that not thinking about
the real nature of floating point data types on computers rises all kind of (future) portability problems.

Currently I see this as a major threat to porting efforts for 32-bit machines (Intel i686, ARM v6/v7):
authors concluding because it's working on their 64-bit Intel laptop at home makes the code correct.
:-)

[DaveDavenport]

Currently I see this as a major threat to porting efforts for 32-bit machines (Intel i686, ARM v6/v7):
authors concluding because it's working on their 64-bit Intel laptop at home makes the code correct.
:-)

Yep. You know what they say about assumptions 😄 . It is easy to make the mistake, even if you are aware of it.

[andreasbaumann]

I didn't mean to be overly snarky. :-)
I'm usually the first one to fall into these kind of bugs myself.

[brarcher]

Adding a warning to the documentation is easy enough to do, if it can help some user avoid pitfalls.

Can you take a look at the following, and see if there is room for improvement?

bf671bf

[andreasbaumann]

This looks good.

I realize now, that doc/check.texi contains tons of nice documentation about floating point
comparision. I just read the header files instead of the documentation first. :-)

[brarcher]

I hear you about reading the header files first, I'll do that sometimes too.

The commit I referenced should help close the documentation loop about using the floating point comparison functions when other asserts may be more correct. Thanks for taking a look.

In case you need it in the future, the API header documentation is available online here and the documentation you mentioned is available in HTML form here.