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

Incorrect formula to compute the epipolar_line in triangulation function #253

Open
lgalardini opened this issue May 25, 2020 · 4 comments
Open

Comments

@lgalardini
Copy link

lgalardini commented May 25, 2020

Hi there and first of all thank you for this library, it is very nice and it contain many interesting algorithm/method.

I came across the Triangulation function and it's core FindOptimalImagePoints, since I was looking for an implementation of the Linstrom algorithm. If I'm right, the implementation in Theia is the niter2, the non-iterative method. I compared against the author's paper and found that there are two errors on line 92 and 93
epipolar_line1 -= e_submatrix * lambda * epipolar_line1;
epipolar_line2 -= e_submatrix.transpose() * lambda * epipolar_line2;

In niter2 implementation, formula 10 and 11 are different, in fact, the code should be:
epipolar_line1 -= e_submatrix * lambda * epipolar_line2;
epipolar_line2 -= e_submatrix.transpose() * lambda * epipolar_line1;

Another question that came in mind is: the author's suggest to use Jenkins-Traub root finding method, have you considered to switch to that? How you manage the root of negative number? In fact, sqrt in this way cannot manage non real roots, maybe I'm doing something wrong but I experienced negative root to be resolved. Maybe it worth to open another issue

@sweeneychris
Copy link
Owner

You could be right. Is it possible to add a unit rest which shows verifies your suggestion? Then I'd be happy to accept a pull request with the fix.

@sweeneychris
Copy link
Owner

Regarding jenkins-traub I actually do have a state of the art implementation of this algorithm (rpoly++) but overall I found the matrix approach to give more stable results consistently, though it's not as fast

@lgalardini
Copy link
Author

lgalardini commented May 25, 2020

You could be right. Is it possible to add a unit rest which shows verifies your suggestion? Then I'd be happy to accept a pull request with the fix.

I'll see if I've time during the next week because I'm very busy right now, maybe someone arrive before me 🥇

I found the matrix approach to give more stable results consistently

What do you mean for "matrix approach"? The sqrt called is not the C++ standard function?
I just checked your rpoly++ implementation, thanks for sharing, that's really interesting, I'll give a try!

@AaronLK
Copy link

AaronLK commented Jun 4, 2021

It looks like this is still in the code, what impact would this bug have on reconstructions?

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

No branches or pull requests

3 participants