Skip to content

Nacho/robust kernel colorized icp#2497

Merged
theNded merged 9 commits intoisl-org:masterfrom
nachovizzo:nacho/robust_kernel_colorized_icp
Oct 20, 2020
Merged

Nacho/robust kernel colorized icp#2497
theNded merged 9 commits intoisl-org:masterfrom
nachovizzo:nacho/robust_kernel_colorized_icp

Conversation

@nachovizzo
Copy link
Copy Markdown
Contributor

@nachovizzo nachovizzo commented Oct 16, 2020

This PR extends PR#2425.

  • I've implemented the robust kernel functionality in the ColoredICP pipeline.
  • To do so, I had to expose the TransformationEstimationForColoredICP. I'm not sure if this was not part of the API for some reason.
  • The new changes are also compatible with existing code(through default parameters)
  • The lambda_geomtric parameters is now expose through the TransformationEstimationForColoredICP class.
  • The registration_colored_icp function has now the exact same API that registration_icp. With the difference that other TransformationEstimation objects are not accepted. These 2 functions could be merge in the future.

There are still some weight = 1.0; That I did not want to inject an explicit robust kernel there. For now, this completes the other PR in terms of the registration pipelines. More to come!


This change is Reviewable

@update-docs
Copy link
Copy Markdown

update-docs Bot commented Oct 16, 2020

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

@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@griegler griegler requested a review from theNded October 17, 2020 06:05
After changing how we compute the Jacbonians API
@germanros1987
Copy link
Copy Markdown
Contributor

Codacy Here is an overview of what got changed by this pull request:

Clones added
============
- cpp/open3d/pipelines/registration/ColoredICP.h  1
         

See the complete overview on Codacy

@nachovizzo
Copy link
Copy Markdown
Contributor Author

@theNded this PR is ready to review. It doesn't add any new functionality, just some changes to the API.
I have also a full implementation of GeneralizedICP that depends on this PR, whenever this gets merged I'll open the gicp PR.

@theNded
Copy link
Copy Markdown
Contributor

theNded commented Oct 20, 2020

@theNded this PR is ready to review. It doesn't add any new functionality, just some changes to the API.
I have also a full implementation of GeneralizedICP that depends on this PR, whenever this gets merged I'll open the gicp PR.

That is awesome! G-ICP is one key feature we are missing, thanks for your great contribution!

Copy link
Copy Markdown
Contributor

@theNded theNded left a comment

Choose a reason for hiding this comment

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

Reviewed 13 of 14 files at r1, 2 of 3 files at r2, 2 of 2 files at r3.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

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.

3 participants