koide3 / ndt_omp

Multi-threaded and SSE friendly NDT algorithm
BSD 2-Clause "Simplified" License
731 stars 321 forks source link

Potentially upstream the improvements to PCL? #19

Open SteveMacenski opened 4 years ago

SteveMacenski commented 4 years ago

Hi,

your readme mentions that it is 10x faster than PCL, any consideration of upstreaming it to PCL so that everyone can share that improvement?

CC @kunaltyagi

kunaltyagi commented 4 years ago

A PR would be welcome.

Please bear in mind a PR before GSoC begins or towards end of GSoC would be preferred. (Reasons being time and a GSoC project on new API to integrate SSE and non SSE code together)

koide3 commented 4 years ago

Hi @SteveMacenski @kunaltyagi , I believe there is no major concerns about upstreaming this module to PCL. Some minor concerns are:

I would like to update the code and make a PR on the PCL repository.

SteveMacenski commented 4 years ago

Awesome! That sounds really great. Thanks for that help, it will go a long way.

SteveMacenski commented 4 years ago

Any movement here?

We're looking at implementing a NDT-MCL algorithm for Navigation2 to replace AMCL and we'd love to make use of this speed up!

SteveMacenski commented 4 years ago

(we're also open for any help / collaboration on that front)

kunaltyagi commented 4 years ago

Current status: One refactor PR is ongoing: https://github.com/PointCloudLibrary/pcl/pull/4180 (waiting on my review, perhaps by this weekend)

One has been merged, and NDT OMP PR hasn't been created yet (IIRC)

koide3 commented 4 years ago

Yes, we have been working on refactoring of the original NDT implementation. After it approved, I'll open a PR for NDT OMP with all the corrections made at PointCloudLibrary/pcl#4180.

SteveMacenski commented 4 years ago

@koide3 what's the status of this? Looks like that PR is in so it should be ready?

koide3 commented 4 years ago

@SteveMacenski I made a PR for the multi-threaded NDT, and probably it will be merged after a few review steps https://github.com/PointCloudLibrary/pcl/pull/4277 I'll inform you here once it's merged