search

PointCloudLibrary / pcl

Point Cloud Library (PCL)
https://pointclouds.org/
Other
9.87k stars 4.61k forks source link

The implementation of parameter score_threshold_ of FPCS algorithm is inconsistent with the declaration #4534

Closed IpandaZll closed 3 years ago

IpandaZll commented 3 years ago

In the "ia_fpcs.h" , there is a parameter called "scorethreshold" , which is declared as follows. The code is in lines 487 through 490.

` /** \brief Score threshold to stop calculation with success.

As described in the code, if the user does not give this parameter, it is set to "approximated overlap". However, in the implementation file("ia_fpcs.hpp"), this parameter is assigned to "1- approximated overlap". As shown in the code below.The code is in lines 315 through 317.

` // set further parameter

if (scorethreshold == FLT_MAX) scorethreshold = 1.f - approxoverlap;`

Did I misunderstand it? I am so confused about this.

mvieth commented 3 years ago

The score threshold is compared with the fitness score of each candidate. The fitness score is calculated with the inlier score (fraction of points that are considered inliers): high inlier score -> good fitness score (close to zero). If there is not much overlap between source and target, there won't be many inliers, even for a good candidate, so it makes sense to make the threshold more loose. On the other hand, if there is a large overlap, there will be more inliers, and the threshold should be stricter.

Does this help?

IpandaZll commented 3 years ago

The score threshold is compared with the fitness score of each candidate. The fitness score is calculated with the inlier score (fraction of points that are considered inliers): high inlier score -> good fitness score (close to zero). If there is not much overlap between source and target, there won't be many inliers, even for a good candidate, so it makes sense to make the threshold more loose. On the other hand, if there is a large overlap, there will be more inliers, and the threshold should be stricter.

Does this help?

Thank you, it helps me a lot ! However, from this point of view, it would be better to change the declaration of the parameter scorethreshold to " scorethreshold depends on the size of the approximated overlap". After all, the phrase "equal to" means that the values of the two should be equal.

mvieth commented 3 years ago

Yes, that would make sense. If you like, open a pull request to fix this.

IpandaZll commented 3 years ago

Yes, that would make sense. If you like, open a pull request to fix this.

Thank you so much! I have opened a pull request for it.