-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Add Symmetric ICP Implementation #7276
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
base: main
Are you sure you want to change the base?
Add Symmetric ICP Implementation #7276
Conversation
Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes. |
d441e69
to
e75b3f7
Compare
I've noticed some failing test cases, but they don't appear to be connected to my recent modifications. Am I responsible for addressing them? |
7c1247f
to
c45b03a
Compare
ebb0809
to
67b735e
Compare
Implements the Symmetric Iterative Closest Point (ICP) algorithm for point cloud registration. This change introduces a new method for aligning point clouds that considers the symmetry between source and target, leading to potentially more accurate and robust registration results, especially when dealing with noisy or incomplete data. Includes C++ and Python implementations with corresponding tests.
Introduces a new class for estimating transformations based on symmetric point-to-plane distance, enhancing registration pipelines. Includes corresponding unit tests to ensure correct functionality and robustness.
Adds more comprehensive tests for the Symmetric ICP registration pipeline, covering various scenarios such as empty correspondence sets, missing normals, and convergence behavior. These improvements enhance the reliability and robustness of the algorithm.
This commit refactors the SymmetricICP class for improved readability and maintainability. It also fixes a potential issue where the kernel_ member was not properly initialized. Changes include: - Ensures proper initialization of the robust kernel. - Improves error logging for clarity. - Modifies the order of methods in the header file for better organization.
Refactors the Symmetric ICP implementation for better readability and consistency. Addresses minor issues in the symmetric transformation estimation and improves code formatting.
Improves the Symmetric ICP estimation by making minor adjustments to variable declarations for better code clarity. The change also includes a cleanup in the header file by removing a redundant public access modifier.
The Eigen/Geometry header was included but not used. Removes the unnecessary include to reduce compilation time and dependencies.
Removes unnecessary includes from SymmetricICP.h and SymmetricICP.cpp, cleaning up the code and reducing compile times.
Adds CUDA implementation for the Symmetric ICP registration algorithm, enabling it to run on GPUs. This improves performance for point cloud registration tasks by leveraging parallel processing capabilities. Includes device consistency tests between CPU and GPU, and supports different robust kernels.
Removes the CLAUDE.md file, as the information it contained is no longer relevant or has been migrated to other documentation.
Fixes an error in the test suite by updating the attribute name used to access correspondence set length. It changes `correspondence_set_` to `correspondence_set`.
Adds normal estimation for both source and target point clouds to ensure compatibility with symmetric ICP. Symmetric ICP requires normals to properly function.
Removes trailing whitespace to improve code consistency and adhere to style guidelines.
Refactors Jacobian calculation for clarity and efficiency. Adds checks for valid correspondences to prevent errors when no valid pairs are available.
67b735e
to
456f14c
Compare
Removes an unnecessary device assertion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a new symmetric ICP algorithm to improve point‐cloud registration by minimizing bidirectional point‐to‐plane errors.
- Introduces
TransformationEstimationSymmetric
andRegistrationSymmetricICP
in both legacy (open3d::pipelines
) and tensor (open3d::t::pipelines
) pipelines. - Implements CPU and CUDA kernels (
ComputePoseSymmetricCPU
/ComputePoseSymmetricCUDA
) and integrates them viaComputePoseSymmetric
. - Adds comprehensive C++ and Python unit tests and updates Python bindings for both pipelines.
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
python/test/test_symmetric_icp.py | New Python tests for legacy pipeline symmetric ICP |
python/test/t/registration/test_registration.py | New Python tests for tensor‐based symmetric ICP |
cpp/open3d/pipelines/registration/SymmetricICP.h/.cpp | Legacy pipeline symmetric ICP declaration & implementation |
cpp/open3d/t/pipelines/registration/TransformationEstimation.h/.cpp | Declares and implements tensor pipeline symmetric estimation |
cpp/open3d/t/pipelines/kernel/RegistrationImpl.h | Declares symmetric ICP CPU/CUDA kernels |
cpp/open3d/t/pipelines/kernel/RegistrationCPU.cpp | Implements symmetric ICP CPU kernel |
cpp/open3d/t/pipelines/kernel/RegistrationCUDA.cu | Implements symmetric ICP CUDA kernel |
cpp/pybind/pipelines/registration/registration.cpp | Binds legacy symmetric ICP to Python |
cpp/pybind/t/pipelines/registration/registration.cpp | Binds tensor symmetric ICP to Python |
cpp/tests/pipelines/registration/SymmetricICP.cpp | C++ tests for legacy pipeline symmetric ICP |
cpp/tests/t/pipelines/registration/TransformationEstimation.cpp | C++ tests for tensor pipeline symmetric estimation |
cpp/tests/t/pipelines/registration/... | Additional tensor‐pipeline C++ tests (device consistency, robust kernels) |
cpp/open3d/pipelines/CMakeLists.txt | Added legacy SymmetricICP sources |
cpp/tests/pipelines/CMakeLists.txt | Added legacy SymmetricICP tests |
cpp/open3d/Open3D.h.in | Exposed SymmetricICP header in main include |
Comments suppressed due to low confidence (1)
python/test/t/registration/test_registration.py:205
- [nitpick] Consider adding a test case to verify that
registration_symmetric_icp
raises an error when the source or target lacks normals in the tensor pipeline, mirroring the legacy behavior enforced inTransformationEstimationSymmetric
.
@pytest.mark.parametrize("device", list_devices())
Co-authored-by: Copilot <[email protected]>
Add Symmetric ICP Implementation for Improved Point Cloud Registration
Type
Motivation and Context
This PR introduces a Symmetric ICP (Iterative Closest Point) algorithm implementation.
Key Benefits:
This implementation follows the symmetric point-to-plane error formulation where the objective function minimizes:
E = Σ [(p_s - p_t) · n_t]² + [(p_s - p_t) · n_s]²
where
p_s
,p_t
are corresponding points andn_s
,n_t
are their respective normals.Checklist:
python util/check_style.py --apply
to apply Open3D code styleto my code.
updated accordingly.
results (e.g. screenshots or numbers) here.
Description
Implementation Details
Core Components:
TransformationEstimationSymmetric
class extendingTransformationEstimation
RegistrationSymmetricICP
function following Open3D's registration API patternsFiles Added/Modified:
cpp/open3d/pipelines/registration/SymmetricICP.h
- Header with class declarationcpp/open3d/pipelines/registration/SymmetricICP.cpp
- Core implementationcpp/tests/pipelines/registration/SymmetricICP.cpp
- Comprehensive C++ testspython/test/test_symmetric_icp.py
- Python test suiteAPI Usage
C++ API:
Python API:
Test Results
C++ Tests (9/9 passing):
[==========] Running 9 tests from 1 test suite.
[----------] 9 tests from SymmetricICP
[ RUN ] SymmetricICP.TransformationEstimationSymmetricConstructor
[ OK ] SymmetricICP.TransformationEstimationSymmetricConstructor (0 ms)
[ RUN ] SymmetricICP.TransformationEstimationSymmetricComputeRMSE
[ OK ] SymmetricICP.TransformationEstimationSymmetricComputeRMSE (0 ms)
[ RUN ] SymmetricICP.TransformationEstimationSymmetricComputeRMSEEmptyCorres
[ OK ] SymmetricICP.TransformationEstimationSymmetricComputeRMSEEmptyCorres (0 ms)
[ RUN ] SymmetricICP.TransformationEstimationSymmetricComputeRMSENoNormals
[ OK ] SymmetricICP.TransformationEstimationSymmetricComputeRMSENoNormals (0 ms)
[ RUN ] SymmetricICP.TransformationEstimationSymmetricComputeTransformation
[ OK ] SymmetricICP.TransformationEstimationSymmetricComputeTransformation (0 ms)
[ RUN ] SymmetricICP.TransformationEstimationSymmetricComputeTransformationEmptyCorres
[ OK ] SymmetricICP.TransformationEstimationSymmetricComputeTransformationEmptyCorres (0 ms)
[ RUN ] SymmetricICP.TransformationEstimationSymmetricComputeTransformationNoNormals
[ OK ] SymmetricICP.TransformationEstimationSymmetricComputeTransformationNoNormals (0 ms)
[ RUN ] SymmetricICP.RegistrationSymmetricICP
[ OK ] SymmetricICP.RegistrationSymmetricICP (36 ms)
[ RUN ] SymmetricICP.RegistrationSymmetricICPConvergence
[ OK ] SymmetricICP.RegistrationSymmetricICPConvergence (4 ms)
[----------] 9 tests from SymmetricICP (41 ms total)
[ PASSED ] 9 tests.
Test Coverage:
Code Quality
Style Compliance:
Design Patterns:
Integration
This implementation integrates seamlessly with Open3D's existing registration pipeline:
The Symmetric ICP method provides users with an additional tool for challenging registration scenarios while maintaining the
familiar Open3D API design.