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

pcl::PCA eigenvectors/eigenvalues are computed with incorrect Cov matrix? (1.8) #2129

Closed
SergioRAgostinho opened this issue Dec 7, 2017 · 0 comments
Assignees
Labels
kind: bug Type of issue

Comments

@SergioRAgostinho
Copy link
Member

Dug this one out of the dev/user list. Original post here with a rebound here.

Hello,

Line 85-88 of: https://github.com/PointCloudLibrary/pcl/blob/pcl-1.8.0/common/include/pcl/common/impl/pca.hpp
/../
Eigen::Matrix3f alpha = static_castEigen::Matrix3f (cloud_demean.topRows<3> () * cloud_demean.topRows<3> ().transpose ());

// Compute eigen vectors and values
Eigen::SelfAdjointEigenSolverEigen::Matrix3f evd (alpha);
/../

alpha is presumably the covariance matrix as it is passed to SelfAdjonitEigenSolver. But shouldn't that be:

alpha = 1/(N-1) * cloud_demean * cloud_demean.transpose()?

Not putting the 1/(N-1) makes the eigenvalues dependent on the number of points you have in the cloud. Double the number of data points you have in the exact same [x,y,z] interval you will get eigenvalues that are twice as big. The eigenvectors may still be fine as they are also normalized. But it still looks incorrect to me.

Was there a reason to write it like this that I'm missing?

Engin

@SergioRAgostinho SergioRAgostinho added needs: code review Specify why not closed/merged yet kind: bug Type of issue and removed needs: code review Specify why not closed/merged yet labels Dec 7, 2017
@SergioRAgostinho SergioRAgostinho self-assigned this Dec 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Type of issue
Projects
None yet
Development

No branches or pull requests

1 participant