PointCloudLibrary / PointCloudLibrary/pcl
Documentation about Registration::setEuclideanFitnessEpsilon() misleading
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 11.1k
- Forks
- 4.7k
- Avg merge
- 4d 10h
- Merged PRs (30d)
- 6
Description
The doc claims that the epsilon refers to the "maximum allowed Euclidean error between two consecutive steps" in the registration. This does not seem to be correct or perhaps not well formulated at least for IterativeClosestPoint, where the value is passed to DefaultConvergenceCriteria where it is used as the relative reduction in MSE between two steps. i.e. a value of 0.01 does not mean "converge when current MSE < 0.01", but rather "converge when reduction in MSE from last iteration is less than 1% of MSE from last iteration".
I'm not sure if the documentation should be updated, since other subclasses may use it differently, or if the entire function should move to the icp class, since only ICP and JointICP seem to actually use it.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with registration/include/pcl/registration/registration.h around line 395, then trace setEuclideanFitnessEpsilon() through DefaultConvergenceCriteria and the IterativeClosestPoint and JointICP subclasses. Check how each subclass uses the value and determine whether the documentation can be made accurate for all callers or needs to be scoped; done means the API documentation no longer misstates the convergence behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- computer-vision
- Issue type
- Documentation
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100