Unit Test Tolerances

[callumcrown]

Description

When running the Unit tests in some 32bit architectures, some of the unit test assert_eaual assertions fail with deltas in the order of 1e-6.

Looking into this further, the expected results in many of the tests (all of the failing tests, but many of the non-failing ones too) are calculated using functions from numericalDerivative.h. By default, the increment for numerical derivatives is 1e-5. All of the tests for the numericalDerivative.h functions have a tolerance of 1e-5. However throughout the rest of the code base, there seems to be an expectation of 1e-9 tolerances.

Is it OK that tests based on code that has only been proven to generate results within a tolerance of 1e-5 are expected to pass with tolerances of 1e-9?

This is not going to be easy to reproduce without our build environment - but I can test any suggestions in our builds.

However, I am more interested in answering whether or not the currently used tolerances make sense than a specific fix.

Failing Tests

gtsam/geometry/tests/testOrientedPlane3.cpp:174: Failure: "assert_equal(H_expected_numerical, H_actual, 1e-9)"
gtsam/geometry/tests/testOrientedPlane3.cpp:180: Failure: "assert_equal(H_expected_numerical, H_actual, 1e-9)"
gtsam/geometry/tests/testPose3.cpp:663: Failure: "assert_equal(expectedH1, actualH1)"
gtsam/geometry/tests/testPose3.cpp:664: Failure: "assert_equal(expectedH2, actualH2)" 
gtsam/geometry/tests/testUnit3.cpp:384: Failure: "assert_equal(H_expected_numerical, H, 1e-9)"

[dellaert]

The increment delta and the tolerance have some relationship but not an obvious one, I think. I don't know immediately how to answer your question, but would welcome a reasoned PR that resolves this on both 32 and 64-bit without compromising correctness.

[jlblancoc]

@callumcrown
I can enable running unit tests on the PPA, so you can see the output for many different architectures and compilers, etc.

I have created a branch unit-test-tolerances, so you can open a PR against that branch, then I'll manually push the branch version to the PPA for testing... sounds like a good idea?

[jlblancoc]

You are right, @callumcrown : unit tests fail in some architectures.
See all PPA builds here, examples:

• amd64 passes
• i386 fails. (Click on "buildlogs" to see full details).

So, feel free of proposing a PR against the branch fix/unit-test-tolerances and we'll test them in all architectures before merging.

[jlblancoc]

Anyone wants to propose a PR to fix this?

If not, I'll try to address it next week.

[callumcrown]

@jlblancoc Thanks a lot for looking into this!

Looking at those build logs, it looks pretty similar to the results in our build environment too. Good job on reproducing the issue! I was worried that it might just be our system.

I can try find some time to look into this again next week, although I don't currently have any ideas other than changing tolerances, which as @dellaert pointed out, isn't ideal...

I wonder if there are any implementation changes that could be made to numericalDerivative.h to make it's results more architecture independent?

[callumcrown]

@jlblancoc unfortunately some last minute travel has come up and I won't be able to look into this until early next year. If no one has time to look into it before then I can pick it back up in January. Sorry about that.

[jlblancoc]

@callumcrown ok, I'm taking care of this one... will PR when tests pass on the PPA.

[jlblancoc]

For the records: As a follow up, there are additional test failures (already trying to fix them):

1. Probably also a numerical (derivative) issue (only fails on i386), but the error is not small:

124/228 Test #124: testAHRSFactor .........................***Failed    0.01 sec
not equal:
expected = [
 	            0             0             0;
    	            0 -2.78687e-309 -7.81412e-311;
    	            0  7.81412e-311 -2.78687e-309
  ]
actual = [
 	        -1.01 -5.15619e-310 -3.09371e-309;
    	-4.62736e-310 -2.47676e-309 -9.16333e-310;
    	-3.15341e-309  9.16333e-310 -2.47676e-309
  ]
actual - expected = [
 	        -1.01 -5.15619e-310 -3.09371e-309;
    	-4.62736e-310  3.10108e-310 -8.38192e-310;
    	-3.15341e-309  8.38192e-310  3.10108e-310
  ]

2. bad_alloc:

158/228 Test #158: testPCGSolver ..........................***Failed    0.00 sec
/<<PKGBUILDDIR>>/tests/testPCGSolver.cpp:129: Failure: "Exception: std::bad_alloc" 
/<<PKGBUILDDIR>>/tests/testPCGSolver.cpp:150: Failure: "Exception: std::bad_alloc" 
/<<PKGBUILDDIR>>/tests/testPCGSolver.cpp:171: Failure: "Exception: std::bad_alloc" 

165/228 Test #165: testSubgraphPreconditioner .............***Failed    0.01 sec
/<<PKGBUILDDIR>>/tests/testSubgraphPreconditioner.cpp:216: Failure: "Exception: std::bad_alloc" 
There were 1 failures

[ProfFan]

@jlblancoc Is the bad_alloc still there for latest develop?

BTW, I think the tolerance failure is probably related to the implementation of numerical solutions: #73 (comment)

[ProfFan]

Also, #183 has unresolved issue of ALIGNED_ALLOC which should be marked resolved if this is closed.

[ProfFan]

@jlblancoc I wonder if you still remember which assertion leads to the numeric error? is it expectedDelRdelBiasOmega, preintegrated.delRdelBiasOmega?

I encountered similar issue in #259, and I failed to reproduce on any of my systems...

Thanks in advance!

[jlblancoc]

@ProfFan sorry, I can't remember right now where exactly was the problem...
I think I tested this on a VirtualBox image to make tests easily, could give it another try with current develop if that's helpful?

PS: we also left open this one (!) #183 ...

[ProfFan]

@jlblancoc I have the exactly same problem (expected should be 1 but becomes 0) in the feature/github_actions_ci branch so I think it has not changed through the months... anything that can reproduce a failure where the error is -1.01 would definitely help!