Skip to content

Support for comparing floats for equality in unit test ==> AssertEqualFloat(expect, actual, abs_delta) ? #244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
RobTillaart opened this issue Dec 12, 2020 · 10 comments · Fixed by #238
Labels
enhancement New feature or request unittest libs The assert / assure / unittest reporting apparatus is affected
Milestone

Comments

@RobTillaart
Copy link

Some of my libraries use float math and errors add up when doing a lot of float math.
e.g https://github.com./RobTillaart/AverageAngle/runs/1543777648?check_suite_focus=true

To test for equality for a float output I typically use the following working construct

  fprintf(stderr, "getTotalLength()\n");
  float diff = abs(10 - aa.getTotalLength());
  assertMoreOrEqual(0.2, diff);

however I would prefer a oneliner

  AssertEqualFloat(10,  aa.getTotalLength(), 0.2);

output should be something like one of the three

AssertEqualFloat  10 == aa.getTotalLength()  +- 0.2
AssertEqualFloat  10 +- 0.2 == aa.getTotalLength()
AssertEqualFloat  10- 0.2 <  aa.getTotalLength() < 10 + 0.2

Maybe it should be called AssertAlmostEqual(...)

Thanks,

@ianfixes
Copy link
Collaborator

Hi Rob- thanks for contributing this.

At the moment, the assertions are really just some thin wrappers around comparisions

I think this will involve some extra macro work; what I have there now was borrowed from work done by @wmacevoy and I'm hoping they can provide a recommendation as to how to proceed here.

@RobTillaart
Copy link
Author

Something like this could work?

#define AssertEqualFloat(arg1, arg2, arg3)  assertOp("assertEqual","expected", fabs(arg1 - arg2),compareEqual,"==","actual",arg3)

@ianfixes ianfixes added enhancement New feature or request unittest libs The assert / assure / unittest reporting apparatus is affected labels Dec 14, 2020
@ianfixes
Copy link
Collaborator

I think you mean compareLessEqual, but yes. The question is whether I should further generalize it by abstracting away the fabs(arg1 - arg2) as a delta function and call the macro assertAlmostEqual or some such. I'm not sure what other types would be relevant for a "equal within delta" calculation, but that's the future-proofing I want to think about here.

@RobTillaart
Copy link
Author

From requirements point of view:

  • I want to compare floats
  • as these can lose bits an AlmostEqual variant is needed
  • Almost equal should have an absolute error as parameter
  • by dividing the absolute error by the expected value one can get a relative error.
  • at the limits of float size there will be problems,

AlmostEqual can of course also be used by

  • integers (any size),
  • Complex numbers ?
  • vectors ?
  • angles - which typically are float or int

Additional - I just realized - I might need

  • equalsINF(float) returns OK if float is +INF or -INF
  • equalsNAN(float) returns OK if float is NAN

@RobTillaart
Copy link
Author

Had a good case to test float (almost) equality with my complex class.

I put the following Macro at top of .....\Complex\test\unit_test_001.cpp

#define assertEqualFloat(arg1, arg2, arg3)  assertOp("assertEqualFloat", "expected", fabs(arg1 - arg2), compareLessOrEqual, "<=", "actual", arg3)

Screenshot:
image

so that works pretty well.

@RobTillaart
Copy link
Author

RobTillaart commented Dec 16, 2020

Got the assertEqualINF and assertEqualNAN working, not needed yet but quite handy.

#define assertEqualFloat(arg1, arg2, arg3)  assertOp("assertEqualFloat", "expected", fabs(arg1 - arg2), compareLessOrEqual, "<=", "actual", arg3)
#define assertEqualINF(arg)  assertOp("assertEqualINF", "expected", INFINITY, compareEqual, "==", "actual", arg)
#define assertEqualNAN(arg)  assertOp("assertEqualNAN", "expected", true, compareEqual, "==", "actual", isnan(arg))

unittest(test_new_operator)
{
  assertEqualINF(exp(800));
  assertEqualINF(0.0/0.0);
  assertEqualINF(42);
  
  assertEqualNAN(INFINITY - INFINITY);
  assertEqualNAN(0.0/0.0);
  assertEqualNAN(42);
}

OUTPUT: (the fail is expected)

# Subtest: test_new_operator
    ok 1 - assertEqualINF (__builtin_inff ()) == exp(800)
    ok 2 - assertEqualINF (__builtin_inff ()) == 0.0/0.0
    not ok 3 - assertEqualINF (__builtin_inff ()) == 42
      ---
      operator: ==
      expected: inf
      actual: 42
      at:
        file: /github/home/Arduino/libraries/Complex/test/unit_test_001.cpp
        line: 46
      ...
    ok 4 - assertEqualNAN true == isnan((__builtin_inff ()) - (__builtin_inff ()))
    ok 5 - assertEqualNAN true == isnan(0.0/0.0)
    not ok 6 - assertEqualNAN true == isnan(42)
      ---
      operator: ==
      expected: 1
      actual: 0
      at:
        file: /github/home/Arduino/libraries/Complex/test/unit_test_001.cpp
        line: 50
      ...
    1..6
not ok 1 - test_new_operator

@RobTillaart
Copy link
Author

RobTillaart commented Dec 16, 2020

Maybe the inverted assertNot_INF and assertNot_NAN are also (even more) interesting.
To be investigated....

@ianfixes
Copy link
Collaborator

assertEqualINF and assertEqualNAN make the case for not bothering to lump in other "almost equal" type of operations. I think the right approach here is to adopt what you are doing.

I should also add docs that say how to use assertOp to define your own behaviors (e.g. maybe comparing a binary tree of some kind where you use a class method instead of the == operator to judge equality).

Have you tried a case where the assertion fails? I'm curious what the error message should like since it involves both the expected value and a delta

@RobTillaart
Copy link
Author

@ianfixes
Is the solution added to the documentation?

@ianfixes
Copy link
Collaborator

ianfixes commented Dec 29, 2020

I put them here https://github.com./Arduino-CI/arduino_ci/blob/master/REFERENCE.md#assertions

Note that these changes haven't been released under a new version of the gem (nor of the GitHub action) yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request unittest libs The assert / assure / unittest reporting apparatus is affected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants