PointCloudLibrary / PointCloudLibrary/pcl
GICP does not use Default Convergence Criteria
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 11.1k
- Forks
- 4.7k
- Avg merge
- 4d 10h
- Merged PRs (30d)
- 6
Description
The implementation of Generalized ICP does not use the DefaultConvergenceCriteria::Ptr convergence_criteria_ This causes confusion when trying to check the state of convergence after running the alignment. Checking the hasConverged() method will return true but checking the state through getConvergenceState() will return ConvergenceState::CONVERGENCE_CRITERIA_NOT_CONVERGED. Because of the conflicting states, it is difficult understand what the state of the convergence is.
A similar issue was brought up for ICP in issue #1598 and addressed by @WraithKim in PR #2892.
Possible Solution
If the using the DefaultConvergenceCriteria does not apply to GICP, the implementation should override getConvergenceCriteria() to throw an exception in order to avoid confusion. Otherwise, the convergence_criteria_ should be updated with the appropriate convergence conditions.
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
Compare Generalized ICP's convergence handling with the ICP change in issue #1598 and PR #2892, focusing on DefaultConvergenceCriteria, hasConverged(), getConvergenceState(), and getConvergenceCriteria(). Determine whether GICP should update convergence_criteria_ or reject getConvergenceCriteria(); done when the reported convergence states no longer conflict and the chosen behavior is covered by relevant tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- computer-vision
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100