kzampog / cilantro

A lean C++ library for working with point cloud data
MIT License
1.02k stars 207 forks source link

openmp errors in visual studio #21

Closed antithing closed 5 years ago

antithing commented 5 years ago

Hi, and thanks for this library! i have the lib built on windows, but am having trouble building the examples. I get OpenMP errors:

Severity    Code    Description Project File    Line    Suppression State
Error   C3001   'declare': expected an OpenMP directive name    rigid_icp   d:\thirdparty\cilantro\include\cilantro\omp_reductions.hpp  9   

Severity    Code    Description Project File    Line    Suppression State
Error   C3036   'max': invalid operator token in OpenMP 'reduction' clause  rigid_icp   d:\thirdparty\cilantro\include\cilantro\icp_warp_field_combined_metric_dense.hpp    154 

Do you have any ideas on what I might do to fix this?

thanks!

kzampog commented 5 years ago

Hi!

Unfortunately, VS still only supports OpenMP 2.0, so user defined reductions (and other things) will not work. I think the easiest solution now would be to completely disable OpenMP for VS builds (at a significant performance loss).

The last time I attempted a VS Windows build was a while ago (#6) and OpenMP did not work (due to OpenMP 2.0 limitations) but did not cause compilation errors. The ones you report must be there because of the reductions introduced in the meantime.

I am not sure if that would be a valid option, but under Windows, building in WSL (Ubuntu) and using a 3rd party X server worked well for me (native Linux performance) in the past. I have not experimented with other compilers yet in Windows.

antithing commented 5 years ago

Hi! The performance is the reason I want to use your lib! Specifically the icp speed increase compared to pcl and open3d. I assume this is because of the openmp functions?

Would it be possible to roll back to an earlier openmp version?

Thanks again

kzampog commented 5 years ago

Performance improvements over PCL are indeed partly due to OpenMP, but that alone is not enough to explain the speedups (the benchmarking in our report was done on a quad core machine), so single threaded performance should also be significantly better. Open3D is also multithreaded with OpenMP. In this case, I would attribute the speedups mostly to nanoflann (Open3D and PCL use FLANN).

Unfortunately, OpenMP never worked in Visual Studio for cilantro (see notes at the bottom of https://github.com/kzampog/cilantro/issues/6#issuecomment-363671898), as we use unsigned loop indices everywhere and OpenMP 2.0 does not work with those. But even in that single threaded case, with optimizations enabled, performance was significantly behind GCC or Clang (tested in Linux with OpenMP disabled).

My first suggestion would then be to try another compiler with modern OpenMP support, as that would most likely improve single threaded performance as well.

If that is not an option and you are only interested in ICP functionality, you could simply comment out all #pragma omp directives that have a reduction clause (to get rid of the compilation errors at a small performance cost) and then modify all remaining parallelized loops to use an int instead of a size_t index in the relevant files. If you were using GCC, I believe this modifications would get you performance similar to what we report. If you would like to try this, I could point you to the files that need to be modified.

antithing commented 5 years ago

Thanks! i have it working now. Commented out the reduction as you suggested, and changed the size_t loops.

I am having trouble getting a good result on my data, but will keep tweaking settings. Thanks!

kzampog commented 5 years ago

Great! If you are using a KDTree-based correspondence search, please make sure the loops in correspondence_search_kd_tree_utilities.hpp are also modified, as this is (by far) the most time consuming step in this scenario.

Let me know if I can be of any further help!