Fix pca_numpy conditioning: SVD the centered data matrix, not the covariance matrix#1524
Open
mengxihex wants to merge 2 commits into
Open
Fix pca_numpy conditioning: SVD the centered data matrix, not the covariance matrix#1524mengxihex wants to merge 2 commits into
mengxihex wants to merge 2 commits into
Conversation
… matrix Forming C = Y.T @ Y / (n - 1) squares the condition number, so near- degenerate inputs (almost collinear point clouds) lose their smallest principal direction to floating-point rounding - bestfit_plane_numpy normals came back 11-37 degrees off on exactly planar sliver clouds. The right-singular vectors of Y are the same eigenvectors; eigenvalues are rescaled (s**2 / (n - 1)) to keep their variance meaning, so well- conditioned results are unchanged. Fixes compas-dev#1522
… no matmul operator)
Author
|
Fixed the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1522.
What
pca_numpyformed the covariance matrixC = Y.T @ Y / (n - 1)and SVD'd it. FormingCsquares the condition number, so for near-degenerate inputs (an almost collinear point cloud whose smallest extent is ~1e-8 of its largest — still an exactly planar, perfectly posed fit) the smallest principal direction drowns at machine epsilon and comes back wrong:bestfit_plane_numpynormals were off by 11.6° (1e-8 aspect) to 37.3° (1e-9 aspect) on the reproduction in #1522.This PR runs the SVD on the centered data matrix
Ydirectly. The right-singular vectors ofYare exactly the eigenvectors ofC, and the singular values are rescaled (s**2 / (n - 1)) so the returned eigenvalues keep their meaning (variance along each principal direction). Well-conditioned inputs return the same results as before; only the ill-conditioned ones improve. Cost is equivalent (both routes are O(n·dim²)).Checks
tests/compas/geometry/test_pca_numpy.py: (1) well-conditioned parity with the covariance route (eigenvalues matcheigvalsh(C), eigenvectors diagonalizeC), (2) the near-collinear cloud recovers its smallest direction to <1e-4°, (3)bestfit_plane_numpyregression on the same cloud (fails on the previous implementation with ~11.6° error).tests/compas/geometrysuite: 519 passed (includes thepca_numpyconsumersbestfit_numpy,bbox_numpy,icp_numpy).One behavioural note for review: SVD sign indeterminacy means individual principal directions may flip sign relative to the old implementation on some inputs (the sign was already unspecified in both routes).