Skip to content

registration: parallelize SAC-IA error metric with OpenMP#6431

Merged
mvieth merged 2 commits intoPointCloudLibrary:masterfrom
zeel2104:sac-ia-openmp-6429
Apr 24, 2026
Merged

registration: parallelize SAC-IA error metric with OpenMP#6431
mvieth merged 2 commits intoPointCloudLibrary:masterfrom
zeel2104:sac-ia-openmp-6429

Conversation

@zeel2104
Copy link
Copy Markdown
Contributor

Summary

This PR parallelizes the error accumulation loop in SampleConsensusInitialAlignment::computeErrorMetric() using OpenMP.

The change uses an OpenMP parallel for with a reduction(+ : error) and thread-local nearest-neighbor buffers via firstprivate(...), so the accumulated registration error can be computed in parallel while preserving the original behavior.

Changes

  • parallelized the per-point loop in registration/include/pcl/registration/impl/ia_ransac.hpp
  • added OpenMP reduction for the error accumulator
  • kept nearest-neighbor index/distance buffers thread-local inside the parallel region

Validation

Built and ran the existing SAC-IA registration test locally on Windows/MSVC:

cmake --build D:\pcl\build --config Release --target test_sac_ia
ctest --test-dir D:\pcl\build\test -C Release -R sac_ia --output-on-failure

@mvieth mvieth added changelog: new feature Meta-information for changelog generation module: registration labels Apr 22, 2026
Copy link
Copy Markdown
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

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

Thank you for the pull request, I have two small comments.

Comment thread registration/include/pcl/registration/impl/ia_ransac.hpp Outdated
Comment thread registration/include/pcl/registration/impl/ia_ransac.hpp Outdated
@mvieth mvieth linked an issue Apr 22, 2026 that may be closed by this pull request
@zeel2104
Copy link
Copy Markdown
Contributor Author

@mvieth
Addressed both comments:

  • changed const auto tree = tree_; to const auto& tree = tree_;
  • added setNumberOfThreads() / threads_ and wired num_threads(threads_) into the SAC-IA OpenMP loop

Validation:

  • cmake --build build --config Release --target test_sac_ia
  • ctest --test-dir build/test -C Release -R sac_ia --output-on-failure
  • cmake --build build --config Release --target pcl_registration

Copy link
Copy Markdown
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@larshg
Copy link
Copy Markdown
Contributor

larshg commented Apr 24, 2026

How much speedup does this give?
Would it be more beneficial to paralellize on the iteration loop instead? The check if a new better transformation has been found ofc needs to be updated in a critical section.

Its just if we add this and later perhaps add the other, we would have nested OMP loops, which probably don't scale that well 😄

@mvieth
Copy link
Copy Markdown
Member

mvieth commented Apr 24, 2026

I did a quick benchmark: 2 threads -> 1.9 speed-up, 3 threads -> 2.6 speed-up, 4 threads -> 3.2 speed-up, 5 threads -> 3.8 speed-up, 6 threads -> 4.4 speed-up. So not perfect, but pretty good.

Parallelizing the outer loop instead would be more difficult, and not that much faster I believe. Not only the updating of the new best transformation would have to be done in an omp-critical section, but also choosing random samples, and probably even more that I am not aware of yet. In contrast, computeErrorMetric is very easy to parallelize, with almost no synchronization overhead, and >90% of the time is spent on that loop.

Its just if we add this and later perhaps add the other, we would have nested OMP loops, which probably don't scale that well

If we ever decide to do that, we could just remove the #pragma omp directive in computeErrorMetric in favour of parallelizing the other loop. But again, I believe parallelizing the loop in computeErrorMetric is the better choice. One thing we could test in the future (not necessarily in this pull request) would be switching to a dynamic schedule, to achieve even better speed-ups.

@mvieth mvieth merged commit 63f25ce into PointCloudLibrary:master Apr 24, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: new feature Meta-information for changelog generation module: registration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[registration] Parallelize SampleConsensusInitialAlignment with OpenMP

3 participants