Bug fix in `SurfaceNormalOutlierFilter`

[cjamin]

This call to anyabs should not be here.

Here is an example: if maxAngle = 0.2, an angle of 3.1 radians should obviously be reject. But with the current code, eps = cos (0.2) ~= 0.98, and abs(cos(3.1) ~= 0.999, so it is accepted. With this fix, cos(3.1 ~= -0.999, it is rejected.

[ethzasl-jenkins]

Can one of the admins verify this patch?

[pomerlef]

ok to test

[pomerlef]

@simonpierredeschenes could you have a look?

[simonpierredeschenes]

I don't think this is the wanted behavior, the call to anyabs is there to allow normals of opposite directions, thus from parallel surfaces. If we remove it, users will need to call the OrientNormalsDataPointsFilter filter to orient all normals in the same direction. This filter acts almost randomly for points far from the robot, so they would likely be discarded by the outlier filter.

[cjamin]

Then the name and this documentation are misleading: "Maximum authorised angle between the 2 surface normals (in radian) - min: 0.0 - max: 3.1416". If it was meant to work like you say, the max would be PI/2, not PI.

I use this filter to prevent matching points that are on two different faces of a wall, so this is really useful.

A solution could be to add a parameter to choose between both behaviors.

[simonpierredeschenes]

@cjamin I discussed with my colleagues and it appears I was mistaken, the filter is meant to avoid matching different faces of a wall, as you said. The fix you did solves the issue. Could you please add a small sentence to the documentation that says that the filter must be used in combination with the OrientNormalsDataPointsFilter to get the expected behavior, to avoid some confusion for the users? Then the PR would be ready to merge. Thanks!

[cjamin]

All right, thanks.

The "official" documentation only mention this filter in this page, without any explanation. This documentation (this link I gave earlier) seems unofficial. So, if I'm not mistaken, I can't change it here in the repository.

[simonpierredeschenes]

Oh, I meant in the outlier filter description (here). This code is used to auto-generate this documentation. This documentation also comes from us, we will link it in the tutorials in the future.

[pomerlef]

The utest are not passing:

Error Message
/home/jenkins/workspace/pointmatcher/label/ubuntu-xenial/utest/utest.cpp:158
Expected: (rel_err) < (0.03), actual: 0.0650631 vs 0.03
This error was caused by the test file:
   ../examples/data/icp_data/force4DOFForPointToPlaneMinimizer.yaml

[cjamin]

I updated the documentation.

Regarding the test, I can only guess that the test used to pass for bad reasons... FYI, I've been testing this patch for a few month on a project now, and it works like a charm.

[pomerlef]

I've updated the test results on your branch. I'll merge as soon as the CI is letting me. Thanks for the patch!

[cjamin]

Thanks for integration!