PointCloudLibrary
Make omp naming and functionality consistent.
An idea: how about, instead of having unsigned int num_threads_{1}; in so many classes, we move that to PCLBase? That would have the additional benefit that there is no ABI break every time parallelization is added to a class (like in the two recent PRs).
As a second step, we could then discuss also moving setNumberOfThreads to PCLBase (to reduce code duplication), however that would make it less clear which classes actually use this setting.
An idea: how about, instead of having
unsigned int num_threads_{1};in so many classes, we move that toPCLBase? That would have the additional benefit that there is no ABI break every time parallelization is added to a class (like in the two recent PRs).As a second step, we could then discuss also moving
setNumberOfThreadstoPCLBase(to reduce code duplication), however that would make it less clear which classes actually use this setting.
Yeah, It seems PCLBase is used for all algorithms, so could make sense. I'm not sure how we should do it with setNumberOfThreads. Not sure if it its enough, just documenting, that not all classes make use of the set value.
Should we default to using omp_get_num_procs() threads instead of 1 - I would expect code to be multithreaded by default 😄
I'm not sure how we should do it with setNumberOfThreads. Not sure if it its enough, just documenting, that not all classes make use of the set value.
I am also fine with setNumberOfThreads staying in each parallelized class.
Should we default to using omp_get_num_procs() threads instead of 1 - I would expect code to be multithreaded by default 😄
Sounds good to me. How are you planning to do that? Calling setNumberOfThreads(0) in each constructor?
Should SampleConsensus inherit from PCLBase as well?
The I/O Imagegrabber also has parallelizing, but should they also inherit from PCLBase?
The Brieft from PCLBase:
PCL base class. Implements methods that are used by most PCL algorithms.
So I guess I/O shouldn't, but SampleConsesus could(Should)?
Should SampleConsensus inherit from PCLBase as well?
The I/O Imagegrabber also has parallelizing, but should they also inherit from PCLBase? The Brieft from PCLBase:
PCL base class. Implements methods that are used by most PCL algorithms.So I guess I/O shouldn't, but SampleConsesus could(Should)?
I think I would not change any inheritance. SampleConsensus and the image grabber can then simply keep their own num_threads_ member.
Rebased to include the update to search https://github.com/PointCloudLibrary/pcl/pull/6284