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

Feaure matching API #5964

Merged
merged 10 commits into from
Apr 4, 2023
Merged

Feaure matching API #5964

merged 10 commits into from
Apr 4, 2023

Conversation

theNded
Copy link
Contributor

@theNded theNded commented Mar 2, 2023

  • Expose correspondences from features api for global registration from external point-wise 3D features (e.g. FCGF).
  • Improve FPFH feature matching time:
Legacy Tensor (CPU) Tensor (CUDA)
0.023 0.016 0.0023

In theory a tensor RANSAC API could be included as well, but there are various problems. Problem analysis step by step.

  1. Correctness: combination check all passed (L for legacy, T for tensor), no problem here:
FPFH feature Feature matching RANSAC Status
L L L ✔️
L T L ✔️
T L L ✔️
T T L ✔️
T T T ✔️
  1. Performance: tensor API is much slower.

Total registration time (s) (100K iterations)

Legacy Tensor (CPU) Tensor (CUDA)
0.088 36.04 35.74

Average run time breakdown (model selection/estimation/check happens every iteration; model validation only happens 250 times per 100K iterations)

Legacy Tensor (CPU) Tensor (CUDA)
Model selection 10^-7 0.002 0.0020
Model estimation 2*10^-6 0.002 0.0022
Model check 10^-7 0.0012 0.0012
Model validation 0.003 0.0024 0.0045

Problem:

  • Model selection/estimation/check: Tensor point cloud operations (ComputeTransformation; SelectByIndex) are only suitable for large inputs. The tensor overhead is overwhelmingly large for small data.
  • Model validation: Hybrid NNS isn't faster for 3D on CUDA

Potential solutions:

  • This PR: Expose feature matching API first and use the legacy RANSAC solver. Tensor RANSAC is temporarily removed and archived in wei/ransac-tensor
  • Future work: Optimized (batch) operations for small point clouds, should consist of several PRs: batch SVD for transformation estimation; batch matrix multiplication for transformation; random tensor generation for model selection.

Type

  • Bug fix (non-breaking change which fixes an issue): Fixes #
  • New feature (non-breaking change which adds functionality). Resolves #
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

Checklist:

  • I have run python util/check_style.py --apply to apply Open3D code style
    to my code.
  • This PR changes Open3D behavior or adds new functionality.
    • Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
      updated accordingly.
    • I have added or updated C++ and / or Python unit tests OR included test
      results
      (e.g. screenshots or numbers) here.
  • I will follow up and update the code if CI fails.
  • For fork PRs, I have selected Allow edits from maintainers.

Description


This change is Reviewable

@update-docs
Copy link

update-docs bot commented Mar 2, 2023

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

@theNded theNded closed this Mar 2, 2023
@theNded theNded reopened this Mar 3, 2023
@theNded theNded marked this pull request as ready for review March 7, 2023 03:56
@theNded theNded requested a review from ssheorey March 13, 2023 15:18
@ShaoxiongYao
Copy link

Thank you for providing this pull request! If I understand correctly, this will enable users to get correspondences from a set of features using the correspondences_from_features, right? With these features, I may be able to implement non-rigid ICP with my customized deformation solver.

@theNded
Copy link
Contributor Author

theNded commented Mar 13, 2023

Thank you for providing this pull request! If I understand correctly, this will enable users to get correspondences from a set of features using the correspondences_from_features, right? With these features, I may be able to implement non-rigid ICP with my customized deformation solver.

Yes, see
https://github.com/isl-org/Open3D/pull/5964/files#diff-6bdcf782f0d76f4eb437bd89ada11445e23a5008633f09131ab9edf8c5806998R118
CUDA acceleration is also supported with the tensor interface, I should update the example in a bit.

cpp/open3d/t/pipelines/registration/Feature.h Show resolved Hide resolved
/// of the aforementioned correspondence set where source[i] and target[j] are
/// mutually the nearest neighbor. If the subset size is smaller than
/// mutual_consistency_ratio * N, return the unfiltered set.
core::Tensor CorrespondencesFromFeatures(const core::Tensor &source_features,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Future PR is OK] Lowe ratio test as an alternative to mutual filter. This has a parameter to allow control over which matches are acceptable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable, will do in a future PR


dst_fpfh_import = o3d.pipelines.registration.Feature()
dst_fpfh_import.data = dst_fpfh_np

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this clearer, can we have a function that computes FPFH features and stores in an npy file. Then another function that reads from that file and uses the new API? A --save-features argument can control this behavior. This can be useful even just with FPFH if we are doing multiway registration [implementation not needed here].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intend to extend this example with support to this functionality. However, there are inconsistencies between tensor point cloud downsample (nearest corner per voxel) and the legacy (mean per voxel). I will make another PR regarding t point cloud, and revisit this example.

cpp/open3d/pipelines/registration/Feature.cpp Outdated Show resolved Hide resolved
@@ -90,6 +90,60 @@ core::Tensor ComputeFPFHFeature(const geometry::PointCloud &input,
return fpfh;
}

core::Tensor CorrespondencesFromFeatures(const core::Tensor &source_features,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these are CUDA tensors, should we copy them to the CPU first?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is no need, as tensor nns backend is able to query properly (and fast). The conversion only has to be done for calling ransac.

cpp/open3d/t/pipelines/registration/Feature.cpp Outdated Show resolved Hide resolved
@guyuezuntinggithub
Copy link

very great work! very appreciate for this.
can I use tensor FPFH in C++ API?

@errissa errissa merged commit a5be78c into master Apr 4, 2023
dbs4261 pushed a commit to dbs4261/Open3D that referenced this pull request Apr 13, 2023
* expose correspondences from features api for external feature usages and debugging

* Improve FPFH feature matching time
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

Successfully merging this pull request may close these issues.

6 participants