From 2b05c57200d462f8956ae805b3f95fe92be9e99d Mon Sep 17 00:00:00 2001 From: alexnavtt Date: Fri, 30 May 2025 16:50:34 -0600 Subject: [PATCH 1/8] changed behavior to index into user indices instead of total pointcloud --- filters/include/pcl/filters/impl/farthest_point_sampling.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp index afdf41aa3f0..7476d8d2284 100644 --- a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp +++ b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp @@ -42,7 +42,7 @@ pcl::FarthestPointSampling::applyFilter (Indices &indices) std::uniform_int_distribution dis(0, size -1); //lambda to map filter indices back to pointcloud indices for increased readability - auto toCloudIndex = [this](const auto idx){return (*indices_)[idx];}; + auto toCloudIndex = [this](const int idx){return (*indices_)[idx];}; //pick the first point at random index_t max_index = dis(random_gen); From 2295d0b4df1653a094a315abccacf45535899dcc Mon Sep 17 00:00:00 2001 From: alexnavtt Date: Sun, 1 Jun 2025 12:14:34 -0600 Subject: [PATCH 2/8] made index mapping lambda take auto parameter --- filters/include/pcl/filters/impl/farthest_point_sampling.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp index 7476d8d2284..afdf41aa3f0 100644 --- a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp +++ b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp @@ -42,7 +42,7 @@ pcl::FarthestPointSampling::applyFilter (Indices &indices) std::uniform_int_distribution dis(0, size -1); //lambda to map filter indices back to pointcloud indices for increased readability - auto toCloudIndex = [this](const int idx){return (*indices_)[idx];}; + auto toCloudIndex = [this](const auto idx){return (*indices_)[idx];}; //pick the first point at random index_t max_index = dis(random_gen); From 8bad84319f0e4cce347a164abd78e9171c2a4d60 Mon Sep 17 00:00:00 2001 From: alexnavtt Date: Mon, 2 Jun 2025 21:20:36 -0600 Subject: [PATCH 3/8] Added parallel implementation of farthest_point_sampling --- .../pcl/filters/farthest_point_sampling.h | 21 +++++++++++++++++++ .../filters/impl/farthest_point_sampling.hpp | 17 +++++++++------ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/filters/include/pcl/filters/farthest_point_sampling.h b/filters/include/pcl/filters/farthest_point_sampling.h index 6089e287da6..d6cba15bb90 100644 --- a/filters/include/pcl/filters/farthest_point_sampling.h +++ b/filters/include/pcl/filters/farthest_point_sampling.h @@ -10,6 +10,7 @@ #pragma once #include +#include #include #include @@ -43,6 +44,7 @@ namespace pcl seed_ (std::random_device()()) { filter_name_ = "FarthestPointSamping"; + setNumberOfThreads(0); } /** \brief Set number of points to be sampled. @@ -79,12 +81,31 @@ namespace pcl return (seed_); } + /** \brief Set the number of threads to use when operating in parallel + * \param nr_threads the number of threads to use + */ + inline void + setNumberOfThreads (unsigned int nr_threads) + { + nr_threads_ = nr_threads == 0 ? omp_get_num_procs() : nr_threads; + } + + /** \brief Get the value of the internal \a nr_threads_ parameter. + */ + inline unsigned int + getNumberOfThreads () const + { + return nr_threads_; + } + protected: /** \brief Number of points that will be returned. */ std::size_t sample_size_; /** \brief Random number seed. */ unsigned int seed_; + /** \brief Number of threads */ + unsigned int nr_threads_; /** \brief Sample of point indices * \param indices indices of the filtered point cloud diff --git a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp index afdf41aa3f0..a8caaaa6be7 100644 --- a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp +++ b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp @@ -51,23 +51,28 @@ pcl::FarthestPointSampling::applyFilter (Indices &indices) for (std::size_t j = 1; j < sample_size_; ++j) { - index_t next_max_index = 0; - const PointT& max_index_point = (*input_)[toCloudIndex(max_index)]; - //recompute distances + + #pragma omp parallel for \ + num_threads(nr_threads_) for (std::size_t i = 0; i < size; ++i) { if (distances_to_selected_points[i] == -1.0) continue; distances_to_selected_points[i] = std::min(distances_to_selected_points[i], geometry::distance((*input_)[toCloudIndex(i)], max_index_point)); - if (distances_to_selected_points[i] > distances_to_selected_points[next_max_index]) - next_max_index = i; + if (distances_to_selected_points[i] > distances_to_selected_points[max_index]) { + #pragma omp critical + { + //have to check the condition again in case another thread modified max_index + if (distances_to_selected_points[i] > distances_to_selected_points[max_index]) + max_index = i; + } + } } //select farthest point based on previously calculated distances //since distance is set to -1 for all selected elements,previously selected //elements are guaranteed to not be selected - max_index = next_max_index; distances_to_selected_points[max_index] = -1.0; indices.push_back(toCloudIndex(max_index)); //set distance to -1 to ignore during max element search From 7f6cff97f2a35190109f208f07a81e53f9bdcd32 Mon Sep 17 00:00:00 2001 From: alexnavtt Date: Mon, 2 Jun 2025 21:26:16 -0600 Subject: [PATCH 4/8] Added `default(none)` to the omp parallel clause --- filters/include/pcl/filters/impl/farthest_point_sampling.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp index a8caaaa6be7..901d61b2ae9 100644 --- a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp +++ b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp @@ -54,6 +54,8 @@ pcl::FarthestPointSampling::applyFilter (Indices &indices) const PointT& max_index_point = (*input_)[toCloudIndex(max_index)]; #pragma omp parallel for \ + default(none) \ + shared(distances_to_selected_points, max_index, max_index_point, size, toCloudIndex) \ num_threads(nr_threads_) for (std::size_t i = 0; i < size; ++i) { From 7f2a4c66c95ae2a04b5067d6a0c497d931d10835 Mon Sep 17 00:00:00 2001 From: alexnavtt Date: Tue, 3 Jun 2025 21:08:25 -0600 Subject: [PATCH 5/8] Fixed first set of review comments --- filters/include/pcl/filters/farthest_point_sampling.h | 9 ++++++--- .../include/pcl/filters/impl/farthest_point_sampling.hpp | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/filters/include/pcl/filters/farthest_point_sampling.h b/filters/include/pcl/filters/farthest_point_sampling.h index d6cba15bb90..767b72381da 100644 --- a/filters/include/pcl/filters/farthest_point_sampling.h +++ b/filters/include/pcl/filters/farthest_point_sampling.h @@ -10,7 +10,6 @@ #pragma once #include -#include #include #include @@ -44,7 +43,6 @@ namespace pcl seed_ (std::random_device()()) { filter_name_ = "FarthestPointSamping"; - setNumberOfThreads(0); } /** \brief Set number of points to be sampled. @@ -87,7 +85,12 @@ namespace pcl inline void setNumberOfThreads (unsigned int nr_threads) { + #ifdef _OPENMP nr_threads_ = nr_threads == 0 ? omp_get_num_procs() : nr_threads; + #else + if (nr_threads_ != 1) + PCL_WARN("OpenMP is not available. Keeping number of threads unchanged at 1\n"); + #endif } /** \brief Get the value of the internal \a nr_threads_ parameter. @@ -105,7 +108,7 @@ namespace pcl /** \brief Random number seed. */ unsigned int seed_; /** \brief Number of threads */ - unsigned int nr_threads_; + unsigned int nr_threads_{1}; /** \brief Sample of point indices * \param indices indices of the filtered point cloud diff --git a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp index 901d61b2ae9..8bb69e10826 100644 --- a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp +++ b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp @@ -57,7 +57,7 @@ pcl::FarthestPointSampling::applyFilter (Indices &indices) default(none) \ shared(distances_to_selected_points, max_index, max_index_point, size, toCloudIndex) \ num_threads(nr_threads_) - for (std::size_t i = 0; i < size; ++i) + for (std::ptrdiff_t i = 0; i < static_cast(size); ++i) { if (distances_to_selected_points[i] == -1.0) continue; From 030910f53044b611fe9971caeece9494831749c3 Mon Sep 17 00:00:00 2001 From: alexnavtt Date: Tue, 3 Jun 2025 21:58:33 -0600 Subject: [PATCH 6/8] Fixed race condition and added separate reduction section --- .../filters/impl/farthest_point_sampling.hpp | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp index 8bb69e10826..b9eac2c7611 100644 --- a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp +++ b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp @@ -53,24 +53,30 @@ pcl::FarthestPointSampling::applyFilter (Indices &indices) { const PointT& max_index_point = (*input_)[toCloudIndex(max_index)]; - #pragma omp parallel for \ + #pragma omp parallel \ default(none) \ shared(distances_to_selected_points, max_index, max_index_point, size, toCloudIndex) \ num_threads(nr_threads_) - for (std::ptrdiff_t i = 0; i < static_cast(size); ++i) { - if (distances_to_selected_points[i] == -1.0) - continue; - distances_to_selected_points[i] = std::min(distances_to_selected_points[i], geometry::distance((*input_)[toCloudIndex(i)], max_index_point)); - if (distances_to_selected_points[i] > distances_to_selected_points[max_index]) { - #pragma omp critical - { - //have to check the condition again in case another thread modified max_index - if (distances_to_selected_points[i] > distances_to_selected_points[max_index]) - max_index = i; + std::ptrdiff_t local_max_index = max_index; + + #pragma omp for + for (std::ptrdiff_t i = 0; i < static_cast(size); ++i) + { + if (distances_to_selected_points[i] == -1.0) + continue; + distances_to_selected_points[i] = std::min(distances_to_selected_points[i], geometry::distance((*input_)[toCloudIndex(i)], max_index_point)); + if (distances_to_selected_points[i] > distances_to_selected_points[local_max_index]) { + local_max_index = i; } } - } + + #pragma omp critical + { + if (distances_to_selected_points[local_max_index] > distances_to_selected_points[max_index]) + max_index = local_max_index; + } + } //pragma omp parallel //select farthest point based on previously calculated distances //since distance is set to -1 for all selected elements,previously selected From c92b5d093053718433857c618fc45e5a4ceac9ff Mon Sep 17 00:00:00 2001 From: alexnavtt Date: Wed, 11 Jun 2025 19:36:35 -0600 Subject: [PATCH 7/8] Renamed nr_threads_ to num_threads_ --- .../include/pcl/filters/farthest_point_sampling.h | 14 +++++++------- .../pcl/filters/impl/farthest_point_sampling.hpp | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/filters/include/pcl/filters/farthest_point_sampling.h b/filters/include/pcl/filters/farthest_point_sampling.h index 767b72381da..8c8a2028b2f 100644 --- a/filters/include/pcl/filters/farthest_point_sampling.h +++ b/filters/include/pcl/filters/farthest_point_sampling.h @@ -80,25 +80,25 @@ namespace pcl } /** \brief Set the number of threads to use when operating in parallel - * \param nr_threads the number of threads to use + * \param num_threads the number of threads to use */ inline void - setNumberOfThreads (unsigned int nr_threads) + setNumberOfThreads (unsigned int num_threads) { #ifdef _OPENMP - nr_threads_ = nr_threads == 0 ? omp_get_num_procs() : nr_threads; + num_threads_ = num_threads == 0 ? omp_get_num_procs() : num_threads; #else - if (nr_threads_ != 1) + if (num_threads_ != 1) PCL_WARN("OpenMP is not available. Keeping number of threads unchanged at 1\n"); #endif } - /** \brief Get the value of the internal \a nr_threads_ parameter. + /** \brief Get the value of the internal \a num_threads_ parameter. */ inline unsigned int getNumberOfThreads () const { - return nr_threads_; + return num_threads_; } protected: @@ -108,7 +108,7 @@ namespace pcl /** \brief Random number seed. */ unsigned int seed_; /** \brief Number of threads */ - unsigned int nr_threads_{1}; + unsigned int num_threads_{1}; /** \brief Sample of point indices * \param indices indices of the filtered point cloud diff --git a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp index b9eac2c7611..1a886880acf 100644 --- a/filters/include/pcl/filters/impl/farthest_point_sampling.hpp +++ b/filters/include/pcl/filters/impl/farthest_point_sampling.hpp @@ -56,7 +56,7 @@ pcl::FarthestPointSampling::applyFilter (Indices &indices) #pragma omp parallel \ default(none) \ shared(distances_to_selected_points, max_index, max_index_point, size, toCloudIndex) \ - num_threads(nr_threads_) + num_threads(num_threads_) { std::ptrdiff_t local_max_index = max_index; From 193f78cc5ded5a9edc4bb19bc3f3cc0964df56f2 Mon Sep 17 00:00:00 2001 From: alexnavtt Date: Thu, 12 Jun 2025 12:31:37 -0600 Subject: [PATCH 8/8] Switched num_threads_ == 0 to num_threads != 0 --- filters/include/pcl/filters/farthest_point_sampling.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filters/include/pcl/filters/farthest_point_sampling.h b/filters/include/pcl/filters/farthest_point_sampling.h index 8c8a2028b2f..0284cc33072 100644 --- a/filters/include/pcl/filters/farthest_point_sampling.h +++ b/filters/include/pcl/filters/farthest_point_sampling.h @@ -86,7 +86,7 @@ namespace pcl setNumberOfThreads (unsigned int num_threads) { #ifdef _OPENMP - num_threads_ = num_threads == 0 ? omp_get_num_procs() : num_threads; + num_threads_ = num_threads != 0 ? num_threads : omp_get_num_procs(); #else if (num_threads_ != 1) PCL_WARN("OpenMP is not available. Keeping number of threads unchanged at 1\n");