Skip to content
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

Better for loops, part 3/N #3556

Merged
merged 8 commits into from
Jan 16, 2020
12 changes: 7 additions & 5 deletions features/include/pcl/features/impl/flare.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,10 @@ template<typename PointInT, typename PointNT, typename PointOutT, typename Signe
//extract support points for the computation of Z axis
std::vector<int> neighbours_indices;
std::vector<float> neighbours_distances;
int n_neighbours = this->searchForNeighbors (index, search_parameter_, neighbours_indices, neighbours_distances);

if (n_neighbours < min_neighbors_for_normal_axis_)
const std::size_t n_normal_neighbours =
this->searchForNeighbors (index, search_parameter_, neighbours_indices, neighbours_distances);
if (n_normal_neighbours < min_neighbors_for_normal_axis_)
{
if (!pcl::isFinite ((*normals_)[index]))
{
Expand Down Expand Up @@ -154,9 +155,10 @@ template<typename PointInT, typename PointNT, typename PointOutT, typename Signe
//find X axis

//extract support points for Rx radius
n_neighbours = sampled_tree_->radiusSearch ((*input_)[index], tangent_radius_, neighbours_indices, neighbours_distances);
const std::size_t n_tangent_neighbours =
sampled_tree_->radiusSearch ((*input_)[index], tangent_radius_, neighbours_indices, neighbours_distances);

if (n_neighbours < min_neighbors_for_tangent_axis_)
if (n_tangent_neighbours < min_neighbors_for_tangent_axis_)
{
//set X axis as a random axis
x_axis = pcl::geometry::randomOrthogonalAxis (fitted_normal);
Expand All @@ -181,7 +183,7 @@ template<typename PointInT, typename PointNT, typename PointOutT, typename Signe

Vector3fMapConst feature_point = (*input_)[index].getVector3fMap ();

for (int curr_neigh = 0; curr_neigh < n_neighbours; ++curr_neigh)
for (std::size_t curr_neigh = 0; curr_neigh < n_tangent_neighbours; ++curr_neigh)
{
const int& curr_neigh_idx = neighbours_indices[curr_neigh];
const float& neigh_distance_sqr = neighbours_distances[curr_neigh];
Expand Down
34 changes: 15 additions & 19 deletions features/include/pcl/features/impl/fpfh.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ pcl::FPFHEstimation<PointInT, PointNT, PointOutT>::computePointSPFHSignature (
float hist_incr = 100.0f / static_cast<float>(indices.size () - 1);

// Iterate over all the points in the neighborhood
for (const int &index : indices)
for (const auto &index : indices)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think auto here decreases readability for the same reasons.

Copy link
Member Author

Choose a reason for hiding this comment

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

auto here enables changing the indices to std::size_t or std::ptrdiff_t later. That change will be quite difficult without minor changes like this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kunaltyagi That's a valid point. But I would argue that the change won't be that difficult with vim + regular expression search and replace. So I would rather prefer keeping good readability.

{
// Avoid unnecessary returns
if (p_idx == index)
Expand Down Expand Up @@ -112,16 +112,16 @@ pcl::FPFHEstimation<PointInT, PointNT, PointOutT>::weightPointSPFHSignature (
float weight = 0.0, val_f1, val_f2, val_f3;

// Get the number of bins from the histograms size
int nr_bins_f1 = static_cast<int> (hist_f1.cols ());
int nr_bins_f2 = static_cast<int> (hist_f2.cols ());
int nr_bins_f3 = static_cast<int> (hist_f3.cols ());
int nr_bins_f12 = nr_bins_f1 + nr_bins_f2;
const auto nr_bins_f1 = hist_f1.cols ();
const auto nr_bins_f2 = hist_f2.cols ();
const auto nr_bins_f3 = hist_f3.cols ();
const auto nr_bins_f12 = nr_bins_f1 + nr_bins_f2;

// Clear the histogram
fpfh_histogram.setZero (nr_bins_f1 + nr_bins_f2 + nr_bins_f3);

// Use the entire patch
for (std::size_t idx = 0, data_size = indices.size (); idx < data_size; ++idx)
for (std::size_t idx = 0; idx < indices.size (); ++idx)
{
// Minus the query point itself
if (dists[idx] == 0)
Expand Down Expand Up @@ -186,10 +186,9 @@ pcl::FPFHEstimation<PointInT, PointNT, PointOutT>::computeSPFHSignatures (std::v
// (We need an SPFH signature for every point that is a neighbor of any point in input_[indices_])
if (surface_ != input_ ||
indices_->size () != surface_->points.size ())
{
for (std::size_t idx = 0; idx < indices_->size (); ++idx)
{
for (const auto& p_idx: *indices_)
{
int p_idx = (*indices_)[idx];
if (this->searchForNeighbors (p_idx, search_parameter_, nn_indices, nn_dists) == 0)
continue;

Expand All @@ -210,13 +209,9 @@ pcl::FPFHEstimation<PointInT, PointNT, PointOutT>::computeSPFHSignatures (std::v
hist_f3.setZero (data_size, nr_bins_f3_);

// Compute SPFH signatures for every point that needs them
std::set<int>::iterator spfh_indices_itr = spfh_indices.begin ();
for (int i = 0; i < static_cast<int> (spfh_indices.size ()); ++i)
std::size_t i = 0;
for (const auto& p_idx: spfh_indices)
{
// Get the next point index
int p_idx = *spfh_indices_itr;
++spfh_indices_itr;

// Find the neighborhood around p_idx
if (this->searchForNeighbors (*surface_, p_idx, search_parameter_, nn_indices, nn_dists) == 0)
continue;
Expand All @@ -226,6 +221,7 @@ 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 @@ -252,14 +248,14 @@ pcl::FPFHEstimation<PointInT, PointNT, PointOutT>::computeFeature (PointCloudOut
{
for (Eigen::Index d = 0; d < fpfh_histogram_.size (); ++d)
output.points[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 (int &nn_index : nn_indices)
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) ...
Expand All @@ -280,14 +276,14 @@ pcl::FPFHEstimation<PointInT, PointNT, PointOutT>::computeFeature (PointCloudOut
{
for (Eigen::Index d = 0; d < fpfh_histogram_.size (); ++d)
output.points[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 (int &nn_index : nn_indices)
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) ...
Expand Down
6 changes: 3 additions & 3 deletions features/include/pcl/features/impl/fpfh_omp.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pcl::FPFHEstimationOMP<PointInT, PointNT, PointOutT>::computeFeature (PointCloud
if (!isFinite ((*input_)[p_idx]) ||
this->searchForNeighbors (p_idx, search_parameter_, nn_indices, nn_dists) == 0)
continue;

spfh_indices_set.insert (nn_indices.begin (), nn_indices.end ());
}
spfh_indices_vec.resize (spfh_indices_set.size ());
Expand All @@ -94,7 +94,7 @@ pcl::FPFHEstimationOMP<PointInT, PointNT, PointOutT>::computeFeature (PointCloud
}

// Initialize the arrays that will store the SPFH signatures
std::size_t data_size = spfh_indices_vec.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_);
Expand Down Expand Up @@ -142,7 +142,7 @@ pcl::FPFHEstimationOMP<PointInT, PointNT, PointOutT>::computeFeature (PointCloud
{
for (int d = 0; d < nr_bins; ++d)
output.points[idx].histogram[d] = std::numeric_limits<float>::quiet_NaN ();

output.is_dense = false;
continue;
}
Expand Down
23 changes: 12 additions & 11 deletions features/include/pcl/features/impl/gfpfh.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,22 +196,24 @@ template <typename PointInT, typename PointNT, typename PointOutT> void
pcl::GFPFHEstimation<PointInT, PointNT, PointOutT>::computeDistanceHistogram (const std::vector<float>& distances,
std::vector<float>& histogram)
{
std::vector<float>::const_iterator min_it = std::min_element (distances.begin (), distances.end ());
assert (min_it != distances.end ());
const float min_value = *min_it;
std::vector<float>::const_iterator min_it, max_it;
std::tie (min_it, max_it) = std::minmax_element (distances.cbegin (), distances.cend ());
assert (min_it != distances.cend ());
assert (max_it != distances.cend ());

std::vector<float>::const_iterator max_it = std::max_element (distances.begin (), distances.end ());
assert (max_it != distances.end());
const float min_value = *min_it;
const float max_value = *max_it;

histogram.resize (descriptorSize (), 0);

const float range = max_value - min_value;
const int max_bin = descriptorSize () - 1;

using binSizeT = decltype(descriptorSize());
const binSizeT max_bin = descriptorSize () - 1;
for (const float &distance : distances)
{
const float raw_bin = static_cast<float> (descriptorSize ()) * (distance - min_value) / range;
int bin = std::min (max_bin, static_cast<int> (std::floor (raw_bin)));
const auto raw_bin = descriptorSize () * (distance - min_value) / range;
const auto bin = std::min<binSizeT> (max_bin, static_cast<binSizeT> (std::floor (raw_bin)));
histogram[bin] += 1;
}
}
Expand Down Expand Up @@ -258,12 +260,11 @@ pcl::GFPFHEstimation<PointInT, PointNT, PointOutT>::getDominantLabel (const std:
counts[label] += 1;
}

std::vector<std::uint32_t>::const_iterator max_it;
max_it = std::max_element (counts.begin (), counts.end ());
const auto max_it = std::max_element (counts.cbegin (), counts.cend ());
if (max_it == counts.end ())
return (emptyLabel ());

return (static_cast<std::uint32_t> (max_it - counts.begin ()));
return std::distance(counts.cbegin (), max_it);
}

#define PCL_INSTANTIATE_GFPFHEstimation(T,NT,OutT) template class PCL_EXPORTS pcl::GFPFHEstimation<T,NT,OutT>;
Expand Down
2 changes: 1 addition & 1 deletion features/include/pcl/features/impl/grsd.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pcl::GRSDEstimation<PointInT, PointNT, PointOutT>::computeFeature (PointCloudOut
rsd.setInputNormals (normals_);
rsd.setRadiusSearch (std::max (search_radius_, std::sqrt (3.0) * width_ / 2));
rsd.compute (*radii);

// Save the type of each point
int NR_CLASS = 5; // TODO make this nicer
std::vector<int> types (radii->points.size ());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,23 +87,27 @@ pcl::MultiscaleFeaturePersistence<PointSource, PointFeature>::initCompute ()
template <typename PointSource, typename PointFeature> void
pcl::MultiscaleFeaturePersistence<PointSource, PointFeature>::computeFeaturesAtAllScales ()
{
features_at_scale_.resize (scale_values_.size ());
features_at_scale_vectorized_.resize (scale_values_.size ());
features_at_scale_.clear ();
features_at_scale_.reserve (scale_values_.size ());
features_at_scale_vectorized_.clear ();
features_at_scale_vectorized_.reserve (scale_values_.size ());
for (std::size_t scale_i = 0; scale_i < scale_values_.size (); ++scale_i)
{
FeatureCloudPtr feature_cloud (new FeatureCloud ());
computeFeatureAtScale (scale_values_[scale_i], feature_cloud);
features_at_scale_[scale_i] = feature_cloud;

// Vectorize each feature and insert it into the vectorized feature storage
std::vector<std::vector<float> > feature_cloud_vectorized (feature_cloud->points.size ());
for (std::size_t feature_i = 0; feature_i < feature_cloud->points.size (); ++feature_i)
std::vector<std::vector<float> > feature_cloud_vectorized;
feature_cloud_vectorized.reserve (feature_cloud->points.size ());

for (const auto& feature: feature_cloud->points)
{
std::vector<float> feature_vectorized (feature_representation_->getNumberOfDimensions ());
feature_representation_->vectorize (feature_cloud->points[feature_i], feature_vectorized);
feature_cloud_vectorized[feature_i] = feature_vectorized;
feature_representation_->vectorize (feature, feature_vectorized);
feature_cloud_vectorized.emplace_back (std::move(feature_vectorized));
}
features_at_scale_vectorized_[scale_i] = feature_cloud_vectorized;
features_at_scale_vectorized_.emplace_back (std::move(feature_cloud_vectorized));
}
}

Expand Down Expand Up @@ -136,9 +140,10 @@ pcl::MultiscaleFeaturePersistence<PointSource, PointFeature>::calculateMeanFeatu
mean_feature_[i] = 0.0f;

float normalization_factor = 0.0f;
for (std::vector<std::vector<std::vector<float> > >::iterator scale_it = features_at_scale_vectorized_.begin (); scale_it != features_at_scale_vectorized_.end(); ++scale_it) {
normalization_factor += static_cast<float> (scale_it->size ());
for (const auto &feature : *scale_it)
for (const auto& scale: features_at_scale_vectorized_)
{
normalization_factor += static_cast<float> (scale.size ());
for (const auto &feature : scale)
for (int dim_i = 0; dim_i < feature_representation_->getNumberOfDimensions (); ++dim_i)
mean_feature_[dim_i] += feature[dim_i];
}
Expand All @@ -152,18 +157,23 @@ pcl::MultiscaleFeaturePersistence<PointSource, PointFeature>::calculateMeanFeatu
template <typename PointSource, typename PointFeature> void
pcl::MultiscaleFeaturePersistence<PointSource, PointFeature>::extractUniqueFeatures ()
{
unique_features_indices_.resize (scale_values_.size ());
unique_features_table_.resize (scale_values_.size ());
unique_features_indices_.clear ();
unique_features_table_.clear ();
unique_features_indices_.reserve (scale_values_.size ());
unique_features_table_.reserve (scale_values_.size ());

for (std::size_t scale_i = 0; scale_i < features_at_scale_vectorized_.size (); ++scale_i)
{
// Calculate standard deviation within the scale
float standard_dev = 0.0;
std::vector<float> diff_vector (features_at_scale_vectorized_[scale_i].size ());
for (std::size_t point_i = 0; point_i < features_at_scale_vectorized_[scale_i].size (); ++point_i)
diff_vector.clear();

for (const auto& feature: features_at_scale_vectorized_[scale_i])
{
float diff = distanceBetweenFeatures (features_at_scale_vectorized_[scale_i][point_i], mean_feature_);
float diff = distanceBetweenFeatures (feature, mean_feature_);
standard_dev += diff * diff;
diff_vector[point_i] = diff;
diff_vector.emplace_back (diff);
}
standard_dev = std::sqrt (standard_dev / static_cast<float> (features_at_scale_vectorized_[scale_i].size ()));
PCL_DEBUG ("[pcl::MultiscaleFeaturePersistence::extractUniqueFeatures] Standard deviation for scale %f is %f\n", scale_values_[scale_i], standard_dev);
Expand All @@ -175,12 +185,12 @@ pcl::MultiscaleFeaturePersistence<PointSource, PointFeature>::extractUniqueFeatu
{
if (diff_vector[point_i] > alpha_ * standard_dev)
{
indices_per_scale.push_back (point_i);
indices_per_scale.emplace_back (point_i);
indices_table_per_scale[point_i] = true;
}
}
unique_features_indices_[scale_i] = indices_per_scale;
unique_features_table_[scale_i] = indices_table_per_scale;
unique_features_indices_.emplace_back (std::move(indices_per_scale));
unique_features_table_.emplace_back (std::move(indices_table_per_scale));
}
}

Expand Down Expand Up @@ -221,16 +231,16 @@ pcl::MultiscaleFeaturePersistence<PointSource, PointFeature>::determinePersisten
}
*/
// Method 2: a feature is considered persistent if it is 'unique' in all the scales
for (std::list<std::size_t>::iterator feature_it = unique_features_indices_.front ().begin (); feature_it != unique_features_indices_.front ().end (); ++feature_it)
for (const auto& feature: unique_features_indices_.front ())
{
bool present_in_all = true;
for (std::size_t scale_i = 0; scale_i < features_at_scale_.size (); ++scale_i)
present_in_all = present_in_all && unique_features_table_[scale_i][*feature_it];
present_in_all = present_in_all && unique_features_table_[scale_i][feature];

if (present_in_all)
{
output_features.points.push_back (features_at_scale_.front ()->points[*feature_it]);
output_indices->push_back (feature_estimator_->getIndices ()->at (*feature_it));
output_features.points.emplace_back (features_at_scale_.front ()->points[feature]);
output_indices->emplace_back (feature_estimator_->getIndices ()->at (feature));
}
}

Expand Down
8 changes: 4 additions & 4 deletions features/include/pcl/features/impl/our_cvfh.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,9 @@ pcl::OURCVFHEstimation<PointInT, PointNT, PointOutT>::sgurf (Eigen::Vector3f & c
s_xplus = s_xminus = s_yplus = s_yminus = 0.f;

//disambiguate rf using all points
for (int k = 0; k < static_cast<int> (grid->points.size ()); k++)
for (const auto& point: grid->points)
{
Eigen::Vector3f pvector = grid->points[k].getVector3fMap ();
Eigen::Vector3f pvector = point.getVector3fMap ();
float dist_x, dist_y;
dist_x = std::abs (evx.dot (pvector));
dist_y = std::abs (evy.dot (pvector));
Expand Down Expand Up @@ -419,9 +419,9 @@ pcl::OURCVFHEstimation<PointInT, PointNT, PointOutT>::computeRFAndShapeDistribut
float sigma = 0.01f; //1cm
float sigma_sq = sigma * sigma;

for (int k = 0; k < static_cast<int> (grid->points.size ()); k++)
for (const auto& point: grid->points)
{
Eigen::Vector4f p = grid->points[k].getVector4fMap ();
Eigen::Vector4f p = point.getVector4fMap ();
p[3] = 0.f;
float d = p.norm ();

Expand Down