Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation about Registration::setEuclideanFitnessEpsilon() misleading #5868

Open
themightyoarfish opened this issue Nov 7, 2023 · 1 comment

Comments

@themightyoarfish
Copy link
Contributor

themightyoarfish commented Nov 7, 2023

/** \brief Set the maximum allowed Euclidean error between two consecutive steps in

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.

@themightyoarfish themightyoarfish changed the title Documentation about Registration::setEuclideanFitnessEpsilon() incorrect Documentation about Registration::setEuclideanFitnessEpsilon() misleading Nov 7, 2023
@mvieth
Copy link
Member

mvieth commented Nov 10, 2023

I agree that the description is not so great. Feel free to suggest a rewording, based on how ICP and JointICP use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants