Fix vector initialization in NormalEstimationOMP
#3614
Conversation
The vectors nn_indices and nn_dists in NormalEstimationOMP::computeFeatures must be initialized to the correct size. They are initialized at the beginning of the function but the OpenMP parallel for loop basically generates a new pair of vectors for each thread. These vectors are initialized with the default constructor which means that they have zero size. To fix this I used 'firstprivate' for the vectors instead of 'private' this causes that the copy constructor is called instead of the default constructor.
|
👍 for As before, it'd be great if you can create a test which fails on OpenMP. I don't know if these issues are tested on the CI, but a test will be a good reason to look into enabling OpenMP on CI in some form |
|
I added the test. |
|
I also fixed the build for Ubuntu 16.04 where there was an error because of some unused variables. Why is the unused parameters warning only treated as an error in Ubuntu 16.04 but not in other builds? |
16.04 has Though I fear windows will be tough to enable the similar flag since the CI spews so much warnings. We might be missing some crucial flag there, due to MSVC's idiosyncrasies. |
|
All tests pass. Please reorganize the commits (squash, rebase as needed) and it's merge time 🚀 |
flowvision
force-pushed
the
vector_initialization_NormalEstimationOMP
branch
from
2107629 to
5e0d70c
Compare
taketwo
changed the title
NormalEstimationOMP
I created a custom pcl::Search class which I then used to do a normal estimation using NormalEstimationOMP. In the implementation of nearestKSearch in my custom class I assumed that k_indices and k_sqr_distances are initialized to the correct size because it says so in the documentation. When I then did the normal estimation the application would crash because I tried to index into an empty vector.
The problem was that the vectors nn_indices and nn_dists in NormalEstimationOMP::computeFeatures must be initialized to the correct size. They are initialized at the beginning of the function but the OpenMP parallel for loop basically generates a new pair of vectors for each thread. These vectors are initialized with the default constructor which means that they have zero size.
To fix this I used 'firstprivate' for the vectors instead of 'private' this causes that the copy constructor to be called instead of the default constructor.