[features] Stabilize ShapeContext3D azimuth orientation#6398
[features] Stabilize ShapeContext3D azimuth orientation#6398zatchbell1311-wq wants to merge 1 commit intoPointCloudLibrary:masterfrom
Conversation
|
Thanks for the pull request, but I have to say, some of the changes seem quite random (especially changes to comments and formatting changes), and some modifications even make the code worse IMO, for example the changes to doxygen comments in 3dsc.h, some documentation is even completely removed??? All of these make in unnecessarily difficult to review this pull request. Please undo all changes except those that fix the specific problem! Additionally, the tests have to be updated to make the CI jobs pass. I think the expected descriptor values in test_shot_estimation.cpp can simply be updated to reflect the new output, but we also need some new test to make sure that the new function which stabilizes the orientation actually does a good job. My suggestion would be: Compute the ShapeContext3D features on a cloud, apply some rotation to the cloud, then compute the features again, and check that they are very similar to the original features (rotation invariance). |
24aee9a to
b32cb1b
Compare
|
Thanks for the review! I've completely reworked the fix—instead of a post-hoc azimuth rotation approach, I now replace the random rnd() x_axis initialization with a deterministic one. The direction to the most distant neighbor is projected onto the tangent plane and used as the x-axis, with a fallback to a fixed global axis for degenerate cases. Only the implementation file changed no formatting or documentation changes this time. Please take another look! |
|
Thanks for your patience. The fix compiles successfully across all platforms. The remaining failures are test expectation values in test_shot_estimation.cpp that need updating to reflect the new deterministic output. Could you advise the best way to obtain the new expected values without a full local build? Happy to update them. |
This PR fixes instability in ShapeContext3D caused by the random azimuth axis selection used during descriptor computation.
Instead of expanding the descriptor with multiple azimuth rotations (as proposed in the original paper), this change applies a deterministic azimuth normalization step after computePoint(). The azimuth block with the highest accumulated energy
is rotated to the first position.
This keeps the descriptor length unchanged (1980) and preserves compatibility with existing PCL feature pipelines while making the output stable across runs.
Fixes #6050