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

BUG in pcl::computeMeanAndCovarianceMatrix #2184

Closed
sourireduclown opened this issue Jan 12, 2018 · 2 comments
Closed

BUG in pcl::computeMeanAndCovarianceMatrix #2184

sourireduclown opened this issue Jan 12, 2018 · 2 comments

Comments

@sourireduclown
Copy link

⚠️ This is a issue tracker, please use our mailing list for questions: www.pcl-users.org. ⚠️

Hi everyone,

I think there is a really old bug in pcl::computeMeanAndCovarianceMatrix. If I use this method, which computes the centroid and the covariance matrix in a single pass, I won't get the same matrix as if I use the pcl::compute3DCentroid followed by pcl::computeCovarianceMatrixNormalized.

in the code there is this note:
* \note This method is theoretically exact. However using float for internal calculations reduces the accuracy but increases the efficiency.

but for me, it is sometimes not accurate at all. I use this to compute the normal of a point, and depending on the method I used the 2 normals computed differ by more than 73°.

I included some code and a small dataset where this happens.

I am not sure why the covariance matrix is computed as is in the code. It might be an approximation but I can't figure out why the 2 methods should be equivalent.

Julien Pennecot

Your Environment

  • Operating System and version: Ubuntu 16.04
  • Compiler: g++ (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609
  • PCL Version: 1.7

Expected Behavior

Current Behavior

Possible Solution

Code to Reproduce

I've attached the code to reproduce and a small data (.pcd) file

Here is what I obtain:
-------- Loaded neighborhood -----------

Method 1 (first centroid then covariance matrix):
Centroid: -84.0436 -6260.34 -872.866 1
Covariance matrix:
2.88933 1.20661 -0.0449026
1.20661 4.96316 0.0020091
-0.0449026 0.0020091 0.00418452
normal: 0.0175065 -0.00466401 0.999836

Method 2 (centroid and covariance matrix together):
Centroid: -84.0436 -6260.34 -872.866 1
Covariance matrix:
2.89258 1.3125 -0.03125
1.3125 8 -4
-0.03125 -4 -0.4375
normal: 0.96043 -0.0698379 0.269622

Difference between 2 methods):
Centroid: 0 0 0 0
Covariance matrix:
0.00324416 0.105885 0.0136526
0.105885 3.03684 -4.00201
0.0136526 -4.00201 -0.441685
Angle between normals: 73.3385

-------- gaussian distribution -----------

Method 1 (first centroid then covariance matrix):
Centroid: -83.8562 -6260.04 -872.006 1
Covariance matrix:
24.7845 -0.61077 -0.0164661
-0.61077 24.204 -0.019987
-0.0164661 -0.019987 0.00989658
normal: -0.000685427 -0.000843415 -0.999999

Method 2 (centroid and covariance matrix together):
Centroid: -83.8562 -6260.04 -872.006 1
Covariance matrix:
24.7939 0 0.015625
0 12 -5.5
0.015625 -5.5 -0.375
normal: 0.000425046 -0.491445 -0.870908

Difference between 2 methods):
Centroid: 0 0 0 0
Covariance matrix:
0.00944519 0.61077 0.0320911
0.61077 -12.204 -5.48001
0.0320911 -5.48001 -0.384897
Angle between normals: 29.3874

Context

main.cpp.txt
neighborhood.pcd.txt

@SergioRAgostinho
Copy link
Member

This a known problem as discussed in here. There's a proposed solution yet to be implemented mentioned here.

Closing it as a duplicate of #560

@sourireduclown
Copy link
Author

OK Thanks for the update.

I think that the user should be able to choose between the single pass and double pass, and this should be available for all algorithms which use this method even if there is a default value for it. There should be also some kind of documentation saying what is happening in both methods and what are the edge cases, (when single pass is OK and when it is not).

Otherwise thanks for the good work,

Julien

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

2 participants