Skip to content

Merge FPFHEstimationOMP into FPFHEstimation #4281

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion features/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ set(impl_incs
"include/pcl/${SUBSYS_NAME}/impl/don.hpp"
"include/pcl/${SUBSYS_NAME}/impl/feature.hpp"
"include/pcl/${SUBSYS_NAME}/impl/fpfh.hpp"
"include/pcl/${SUBSYS_NAME}/impl/fpfh_omp.hpp"
"include/pcl/${SUBSYS_NAME}/impl/gasd.hpp"
"include/pcl/${SUBSYS_NAME}/impl/gfpfh.hpp"
"include/pcl/${SUBSYS_NAME}/impl/integral_image2D.hpp"
Expand Down
62 changes: 27 additions & 35 deletions features/include/pcl/features/fpfh.h
Original file line number Diff line number Diff line change
@@ -1,41 +1,10 @@
/*
* Software License Agreement (BSD License)
* SPDX-License-Identifier: BSD-3-Clause
*
* Point Cloud Library (PCL) - www.pointclouds.org
* Copyright (c) 2010-2011, Willow Garage, Inc.
* Copyright (c) 2012-, Open Perception, Inc.
*
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
*
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided
* with the distribution.
* * Neither the name of the copyright holder(s) nor the names of its
* contributors may be used to endorse or promote products derived
* from this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*
* $Id$
*
*/

#pragma once
Expand Down Expand Up @@ -93,9 +62,10 @@ namespace pcl
using PointCloudOut = typename Feature<PointInT, PointOutT>::PointCloudOut;

/** \brief Empty constructor. */
FPFHEstimation () :
FPFHEstimation (unsigned int nr_threads = -1) :
nr_bins_f1_ (11), nr_bins_f2_ (11), nr_bins_f3_ (11),
d_pi_ (1.0f / (2.0f * static_cast<float> (M_PI)))
d_pi_ (1.0f / (2.0f * static_cast<float> (M_PI))),
threads_(nr_threads)
{
feature_name_ = "FPFHEstimation";
};
Expand Down Expand Up @@ -177,6 +147,21 @@ namespace pcl
nr_bins_f3 = nr_bins_f3_;
}

/** \brief Initialize the scheduler and set the number of threads to use.
* \param[in] nr_threads the number of hardware threads to use (0 sets the value back to automatic)
*/
void
setNumberOfThreads (unsigned int nr_threads = 0) {
if (nr_threads == 0)
#ifdef _OPENMP
threads_ = omp_get_num_procs();
#else
threads_ = 1;
#endif
else
threads_ = nr_threads;
}

protected:

/** \brief Estimate the set of all SPFH (Simple Point Feature Histograms) signatures for the input cloud
Expand Down Expand Up @@ -213,8 +198,15 @@ namespace pcl
Eigen::VectorXf fpfh_histogram_;

/** \brief Float constant = 1.0 / (2.0 * M_PI) */
float d_pi_;
float d_pi_;

/** \brief The number of threads the scheduler should use. */
unsigned int threads_;
};

template <typename PointInT, typename PointNT, typename PointOutT>
using FPFHEstimationOMP PCL_DEPRECATED(1, 12, "use FPFHEstimation instead") = FPFHEstimation<PointInT, PointNT, PointOutT>;

}

#ifdef PCL_NO_PRECOMPILE
Expand Down
87 changes: 2 additions & 85 deletions features/include/pcl/features/fpfh_omp.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,91 +40,8 @@

#pragma once

#include <pcl/features/feature.h>
#include <pcl/features/fpfh.h>

namespace pcl
{
/** \brief FPFHEstimationOMP estimates the Fast Point Feature Histogram (FPFH) descriptor for a given point cloud
* dataset containing points and normals, in parallel, using the OpenMP standard.
*
* \note If you use this code in any academic work, please cite:
*
* - R.B. Rusu, N. Blodow, M. Beetz.
* Fast Point Feature Histograms (FPFH) for 3D Registration.
* In Proceedings of the IEEE International Conference on Robotics and Automation (ICRA),
* Kobe, Japan, May 12-17 2009.
* - R.B. Rusu, A. Holzbach, N. Blodow, M. Beetz.
* Fast Geometric Point Labeling using Conditional Random Fields.
* In Proceedings of the 22nd IEEE/RSJ International Conference on Intelligent Robots and Systems (IROS),
* St. Louis, MO, USA, October 11-15 2009.
*
* \attention
* The convention for FPFH features is:
* - if a query point's nearest neighbors cannot be estimated, the FPFH feature will be set to NaN
* (not a number)
* - it is impossible to estimate a FPFH descriptor for a point that
* doesn't have finite 3D coordinates. Therefore, any point that contains
* NaN data on x, y, or z, will have its FPFH feature property set to NaN.
*
* \author Radu B. Rusu
* \ingroup features
*/
template <typename PointInT, typename PointNT, typename PointOutT>
class FPFHEstimationOMP : public FPFHEstimation<PointInT, PointNT, PointOutT>
{
public:
using Ptr = shared_ptr<FPFHEstimationOMP<PointInT, PointNT, PointOutT> >;
using ConstPtr = shared_ptr<const FPFHEstimationOMP<PointInT, PointNT, PointOutT> >;
using Feature<PointInT, PointOutT>::feature_name_;
using Feature<PointInT, PointOutT>::getClassName;
using Feature<PointInT, PointOutT>::indices_;
using Feature<PointInT, PointOutT>::k_;
using Feature<PointInT, PointOutT>::search_parameter_;
using Feature<PointInT, PointOutT>::input_;
using Feature<PointInT, PointOutT>::surface_;
using FeatureFromNormals<PointInT, PointNT, PointOutT>::normals_;
using FPFHEstimation<PointInT, PointNT, PointOutT>::hist_f1_;
using FPFHEstimation<PointInT, PointNT, PointOutT>::hist_f2_;
using FPFHEstimation<PointInT, PointNT, PointOutT>::hist_f3_;
using FPFHEstimation<PointInT, PointNT, PointOutT>::weightPointSPFHSignature;

using PointCloudOut = typename Feature<PointInT, PointOutT>::PointCloudOut;

/** \brief Initialize the scheduler and set the number of threads to use.
* \param[in] nr_threads the number of hardware threads to use (0 sets the value back to automatic)
*/
FPFHEstimationOMP (unsigned int nr_threads = 0) : nr_bins_f1_ (11), nr_bins_f2_ (11), nr_bins_f3_ (11)
{
feature_name_ = "FPFHEstimationOMP";
PCL_DEPRECATED_HEADER(1, 13, "Use <pcl/features/fpfh.h> instead.")
Copy link
Member

Choose a reason for hiding this comment

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

The number here is for removal. Release in 1.12 and removal in 1.13 doesn't sound great. Make it the 1.14 at least


setNumberOfThreads(nr_threads);
}

/** \brief Initialize the scheduler and set the number of threads to use.
* \param[in] nr_threads the number of hardware threads to use (0 sets the value back to automatic)
*/
void
setNumberOfThreads (unsigned int nr_threads = 0);

private:
/** \brief Estimate the Fast Point Feature Histograms (FPFH) descriptors at a set of points given by
* <setInputCloud (), setIndices ()> using the surface in setSearchSurface () and the spatial locator in
* setSearchMethod ()
* \param[out] output the resultant point cloud model dataset that contains the FPFH feature estimates
*/
void
computeFeature (PointCloudOut &output) override;
#include <pcl/features/fpfh.h>

public:
/** \brief The number of subdivisions for each angular feature interval. */
int nr_bins_f1_, nr_bins_f2_, nr_bins_f3_;
private:
/** \brief The number of threads the scheduler should use. */
unsigned int threads_;
};
}

#ifdef PCL_NO_PRECOMPILE
#include <pcl/features/impl/fpfh_omp.hpp>
#endif
127 changes: 40 additions & 87 deletions features/include/pcl/features/impl/fpfh.hpp
Original file line number Diff line number Diff line change
@@ -1,41 +1,10 @@
/*
* Software License Agreement (BSD License)
* SPDX-License-Identifier: BSD-3-Clause
*
* Point Cloud Library (PCL) - www.pointclouds.org
* Copyright (c) 2010-2011, Willow Garage, Inc.
* Copyright (c) 2012-, Open Perception, Inc.
*
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
*
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided
* with the distribution.
* * Neither the name of the copyright holder(s) nor the names of its
* contributors may be used to endorse or promote products derived
* from this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*
* $Id$
*
*/

#pragma once
Expand Down Expand Up @@ -186,7 +155,8 @@ pcl::FPFHEstimation<PointInT, PointNT, PointOutT>::computeSPFHSignatures (std::v
std::vector<int> nn_indices (k_);
std::vector<float> nn_dists (k_);

std::set<int> spfh_indices;
std::set<int> spfh_indices_set;
std::vector<int> spfh_indices_vec;
spfh_hist_lookup.resize (surface_->size ());

// Build a list of (unique) indices for which we will need to compute SPFH signatures
Expand All @@ -199,26 +169,37 @@ pcl::FPFHEstimation<PointInT, PointNT, PointOutT>::computeSPFHSignatures (std::v
if (this->searchForNeighbors (p_idx, search_parameter_, nn_indices, nn_dists) == 0)
continue;

spfh_indices.insert (nn_indices.begin (), nn_indices.end ());
spfh_indices_set.insert (nn_indices.begin (), nn_indices.end ());
}
spfh_indices_vec.resize (spfh_indices_set.size ());
std::copy (spfh_indices_set.cbegin (), spfh_indices_set.cend (), spfh_indices_vec.begin ());
}
else
{
// Special case: When a feature must be computed at every point, there is no need for a neighborhood search
for (std::size_t idx = 0; idx < indices_->size (); ++idx)
spfh_indices.insert (static_cast<int> (idx));
spfh_indices_vec.resize (indices_->size ());
std::iota(spfh_indices_vec.begin (), spfh_indices_vec.end (),
static_cast<decltype(spfh_indices_vec)::value_type>(0));
}

// Initialize the arrays that will store the SPFH signatures
std::size_t data_size = spfh_indices.size ();
const auto data_size = spfh_indices_vec.size ();
hist_f1.setZero (data_size, nr_bins_f1_);
hist_f2.setZero (data_size, nr_bins_f2_);
hist_f3.setZero (data_size, nr_bins_f3_);

// Compute SPFH signatures for every point that needs them
std::size_t i = 0;
for (const auto& p_idx: spfh_indices)
#pragma omp parallel for \
default(none) \
shared(spfh_hist_lookup, spfh_indices_vec, hist_f1, hist_f2, hist_f3) \
firstprivate(nn_indices, nn_dists) \
num_threads(threads_) \
if (threads_ != -1)
for (std::ptrdiff_t i = 0; i < static_cast<std::ptrdiff_t> (spfh_indices_vec.size ()); ++i)
{
// Get the next point index
int p_idx = spfh_indices_vec[i];

// Find the neighborhood around p_idx
if (this->searchForNeighbors (*surface_, p_idx, search_parameter_, nn_indices, nn_dists) == 0)
continue;
Expand All @@ -228,7 +209,6 @@ pcl::FPFHEstimation<PointInT, PointNT, PointOutT>::computeSPFHSignatures (std::v

// Populate a lookup table for converting a point index to its corresponding row in the spfh_hist_* matrices
spfh_hist_lookup[p_idx] = i;
i++;
}
}

Expand All @@ -245,59 +225,32 @@ pcl::FPFHEstimation<PointInT, PointNT, PointOutT>::computeFeature (PointCloudOut
computeSPFHSignatures (spfh_hist_lookup, hist_f1_, hist_f2_, hist_f3_);

output.is_dense = true;
// Save a few cycles by not checking every point for NaN/Inf values if the cloud is set to dense
if (input_->is_dense)
{
// Iterate over the entire index vector
for (std::size_t idx = 0; idx < indices_->size (); ++idx)
{
if (this->searchForNeighbors ((*indices_)[idx], search_parameter_, nn_indices, nn_dists) == 0)
{
for (Eigen::Index d = 0; d < fpfh_histogram_.size (); ++d)
output[idx].histogram[d] = std::numeric_limits<float>::quiet_NaN ();

output.is_dense = false;
continue;
}

// ... and remap the nn_indices values so that they represent row indices in the spfh_hist_* matrices
// instead of indices into surface_->points
for (auto &nn_index : nn_indices)
nn_index = spfh_hist_lookup[nn_index];

// Compute the FPFH signature (i.e. compute a weighted combination of local SPFH signatures) ...
weightPointSPFHSignature (hist_f1_, hist_f2_, hist_f3_, nn_indices, nn_dists, fpfh_histogram_);

// ...and copy it into the output cloud
std::copy_n(fpfh_histogram_.data (), fpfh_histogram_.size (), output[idx].histogram);
}
}
else
// Iterate over the entire index vector
for (std::size_t idx = 0; idx < indices_->size (); ++idx)
{
// Iterate over the entire index vector
for (std::size_t idx = 0; idx < indices_->size (); ++idx)
{
if (!isFinite ((*input_)[(*indices_)[idx]]) ||
this->searchForNeighbors ((*indices_)[idx], search_parameter_, nn_indices, nn_dists) == 0)
{
for (Eigen::Index d = 0; d < fpfh_histogram_.size (); ++d)
output[idx].histogram[d] = std::numeric_limits<float>::quiet_NaN ();

output.is_dense = false;
if (input_->is_dense || isFinite ((*input_)[(*indices_)[idx]])) {
Copy link
Member

Choose a reason for hiding this comment

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

Please benchmark this. Should not have adverse effects, but just to be sure

if (this->searchForNeighbors(
Copy link
Member

Choose a reason for hiding this comment

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

Outdent the block please

(*indices_)[idx], search_parameter_, nn_indices, nn_dists)) {
// ... and remap the nn_indices values so that they represent row indices in the spfh_hist_* matrices
// instead of indices into surface_->points
for (auto &nn_index : nn_indices)
nn_index = spfh_hist_lookup[nn_index];

// Compute the FPFH signature (i.e. compute a weighted combination of local SPFH signatures) ...
weightPointSPFHSignature (hist_f1_, hist_f2_, hist_f3_, nn_indices, nn_dists, fpfh_histogram_);

// ...and copy it into the output cloud
std::copy_n(fpfh_histogram_.data (), fpfh_histogram_.size (), output[idx].histogram);
continue;
}
}

// ... and remap the nn_indices values so that they represent row indices in the spfh_hist_* matrices
// instead of indices into surface_->points
for (auto &nn_index : nn_indices)
nn_index = spfh_hist_lookup[nn_index];

// Compute the FPFH signature (i.e. compute a weighted combination of local SPFH signatures) ...
weightPointSPFHSignature (hist_f1_, hist_f2_, hist_f3_, nn_indices, nn_dists, fpfh_histogram_);
for (Eigen::Index d = 0; d < fpfh_histogram_.size(); ++d)
output[idx].histogram[d] = std::numeric_limits<float>::quiet_NaN();

// ...and copy it into the output cloud
std::copy_n(fpfh_histogram_.data (), fpfh_histogram_.size (), output[idx].histogram);
}
if (output.is_dense)
output.is_dense = false;
}
}

Expand Down
Loading