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

RMSE Calculation in EvaluateRANSACBasedOnDistance Method #6291

Closed
3 tasks done
pherrusa7 opened this issue Aug 2, 2023 · 4 comments · Fixed by #6305
Closed
3 tasks done

RMSE Calculation in EvaluateRANSACBasedOnDistance Method #6291

pherrusa7 opened this issue Aug 2, 2023 · 4 comments · Fixed by #6305
Assignees
Labels
bug Not a build issue, this is likely a bug. registration

Comments

@pherrusa7
Copy link

Checklist

My Question

Hello everyone,

First of all, thank you for your contributions to this great library!

This question might or might not turn into a minor bug. I was reading the code for EvaluateRANSACBasedOnDistance and I found the following formula:
result.inlier_rmse_ = error / std::sqrt((double)inlier_num);

However, error is collected by adding absolute values (as opposed to squared):

double distance = std::abs(plane_model.dot(point));
...
error += distance;

Given how the error is accumulated I was expecting to find a mean absolute error. Is there any reason for the denominator to be the square root of the sample size rather than just the sample size? I might be missing something (or maybe it is indeed a minor bug) so any ideas are welcome.

Thank you for your time :)

@theNded
Copy link
Contributor

theNded commented Aug 10, 2023

This is a good catch! I think it is a bug. I will mark it on my TODO list and fix it.

@theNded theNded added registration bug Not a build issue, this is likely a bug. and removed question labels Aug 10, 2023
@theNded theNded self-assigned this Aug 10, 2023
@pherrusa7
Copy link
Author

pherrusa7 commented Aug 10, 2023

Glad to be helpful! I'd be happy to do a PR with some guidance. Specifically, do you think the error should be squared during accumulation or averaged later by the number of samples? Also, is there a reference paper based on which this routine was implemented?

@theNded
Copy link
Contributor

theNded commented Aug 11, 2023

They should be squared. I think it is relatively a trivial problem now and can be traced back to 2000s. MATLAB has a reference list: https://www.mathworks.com/help/vision/ref/pcfitplane.html

@theNded theNded mentioned this issue Aug 11, 2023
9 tasks
@pherrusa7
Copy link
Author

Yes, that makes sense. I see that you already made the changes so it is done. Thank you!

theNded added a commit that referenced this issue Aug 15, 2023
* fix 6291

* fix 6236

* address review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Not a build issue, this is likely a bug. registration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants