Skip to content

Circle3D: fix optimizeModelCoefficients use an uninitialized var#6088

Merged
mvieth merged 2 commits intoPointCloudLibrary:masterfrom
sunbuny:dev/sunbin
Jul 19, 2024
Merged

Circle3D: fix optimizeModelCoefficients use an uninitialized var#6088
mvieth merged 2 commits intoPointCloudLibrary:masterfrom
sunbuny:dev/sunbin

Conversation

@sunbuny
Copy link
Copy Markdown
Contributor

@sunbuny sunbuny commented Jul 17, 2024

pcl::SampleConsensusModelCircle3D::optimizeModelCoefficients use a uninitialized var as a initial guess to do the minimization. It will never get a correct answer, and the minimization will never be done. This PR fix it.

@mvieth
Copy link
Copy Markdown
Member

mvieth commented Jul 17, 2024

Thanks for fixing this! I think the last three coefficients, that represent the normal direction, also need to be normalized after the minimization, like so: coeff.tail<3>().normalize();. Could you add that as well, please?

@mvieth mvieth added module: sample_consensus changelog: fix Meta-information for changelog generation labels Jul 17, 2024
@sunbuny
Copy link
Copy Markdown
Contributor Author

sunbuny commented Jul 18, 2024

Thanks for fixing this! I think the last three coefficients, that represent the normal direction, also need to be normalized after the minimization, like so: coeff.tail<3>().normalize();. Could you add that as well, please?

You are right, the axis is not nomalized after the minimization. This will not affect the result, but will confuse the user.

Copy link
Copy Markdown
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mvieth mvieth merged commit f48626a into PointCloudLibrary:master Jul 19, 2024
@mvieth mvieth changed the title fix optimizeModelCoefficients use a uninitialized var when do optimization Circle3D: fix optimizeModelCoefficients use an uninitialized var Dec 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: fix Meta-information for changelog generation module: sample_consensus

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants