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

Fix bug in eigenvalues classifying points #119

Merged
merged 1 commit into from
Jan 25, 2019

Conversation

jlblancoc
Copy link
Contributor

Eigen::SelfAdjointEigenSolver docs say: Only the lower triangular part of the input matrix is referenced.
The original code was filling the upper triangular part.
As a result, the obtained eigenvalues were actually... the diagonal elements, while totally ignoring their correlation.

In practice: the original code was not doing its job while classifying point patches that were not at 90 degrees with the (arbitrary) XYZ axes.

Closes #118

Eigen::SelfAdjointEigenSolver docs say: Only the lower triangular part of the input matrix is referenced.
The original code was filling the upper triangular part. 
As a result, the obtained eigenvalues were actually... the diagonal elements, while totally ignoring their correlation. 

In practice: the original code was not doing its job while classifying point patches that were not at 90 degrees with the (arbitrary) XYZ axes.
@laboshinl laboshinl merged commit 25db5dd into laboshinl:master Jan 25, 2019
@ojura
Copy link

ojura commented Jan 30, 2019

Hi, a question: why not do a JacobiSVD on the 3x5 data matrix with the subtracted centroid from every column? The first column of U contains the dominant line, while the third contains the normal. This is also the method described in the original loam paper.

@jlblancoc
Copy link
Contributor Author

Hmm.... what 3x5 matrix? I guess you meant 3x3, right?
You're right, the SVD method would also work, and is probably more numerically stable, but my patch just fixed the original LOAM code which uses Eigen::SelfAdjointEigenSolver :-)

Anyway, it should be benchmarked to decide whether it really represents a significant improvement in either speed or accuracy...

@ojura
Copy link

ojura commented Feb 2, 2019

Nope, 3x5 :) 5 points are drawn from the kd tree to which we want to fit a line or a plane. If you stack them in columns, you get a 3 (xyz) by 5 (column for each point) data matrix. Compute and subtract the centroid from each column and fire up SVD. The result is really elegant - the 3x3 U of the decomposition has the line direction and plane normal as the first and third columns.

More details in
https://www.ltu.se/cms_fs/1.51590!/svd-fitting.pdf

@jlblancoc
Copy link
Contributor Author

Ah, ok now I got your idea! Sure, it's a good approach... should be benchmarked to check if it's more or less efficient than the current one...

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

Successfully merging this pull request may close these issues.

3 participants