[NormalEstimationOMP] appearantly running with just one thread

[mynickmynick]

Describe the bug

Apperantly NormalEstimationOMP runs with just one thread on two different PCs I tested it
I guess it's not a bug of PCL and it's my fault somewhere, I cannot figure out where

Context

I was just making a test to benchmark on my PC what speed up would be given by NormalEstimationOMP from NormalEstimation
but it seems no speed up, so I investigated further and by ouputting
int total_td = omp_get_num_threads();
printf("omp_get_num_threads %d \n", total_td);

i get 1

I tried it also on another pc and the same result

I wonder what am i doing wrong, in any case some sort of warning should be given

The compile config. WITH_OPENMP is true

I even added to CMakeLists to debug

if(OpenMP_FOUND)
message(STATUS "----------!!!!! OpenMP IS ACTIVE !!!!! OpenMP_FOUND: ${OpenMP_FOUND}")
message(STATUS "----------!!!!! OpenMP IS ACTIVE !!!!! OpenMP_VERSION: ${OpenMP_VERSION}")
message(STATUS " OpenMP_C_FOUND: ${OpenMP_C_FOUND}")
message(STATUS " OpenMP_CXX_FOUND: ${OpenMP_CXX_FOUND}")
message(STATUS " OpenMP_INCLUDE_DIRS: ${OpenMP_INCLUDE_DIRS}")
message(STATUS " OpenMP_C_INCLUDE_DIRS: ${OpenMP_C_INCLUDE_DIRS}")
message(STATUS " OpenMP_CXX_INCLUDE_DIRS: ${OpenMP_CXX_INCLUDE_DIRS}")
message(STATUS " OPENMP_INCLUDES: ${OPENMP_INCLUDES}")
message(STATUS " OPENMP_LIBRARIES: ${OPENMP_LIBRARIES}")

and I get

1> [CMake] -- ----------!!!!! OpenMP IS ACTIVE !!!!! OpenMP_FOUND: TRUE
1> [CMake] -- ----------!!!!! OpenMP IS ACTIVE !!!!! OpenMP_VERSION:
1> [CMake] -- OpenMP_C_FOUND: TRUE
1> [CMake] -- OpenMP_CXX_FOUND: TRUE
1> [CMake] -- OpenMP_INCLUDE_DIRS:
1> [CMake] -- OpenMP_C_INCLUDE_DIRS:
1> [CMake] -- OpenMP_CXX_INCLUDE_DIRS:
1> [CMake] -- OPENMP_INCLUDES:
1> [CMake] -- OPENMP_LIBRARIES:
1> [CMake] -- Found OpenMP, spec date 200203

I don't know if those empty strings are a bad sign, I guess so but what to do??

I have also tried to add openMP pragmas to another for elsewhere in my code and it does the same (just one thread) so it must be a problem of my environment

Expected behavior

a speed up of the processing and more than one thread used and logged via omp_get_num_threads

I don't understand because I think I used openMP not so long ago, I wonder if it is some wrong setting on my PC (but it's two PCs)

Your Environment (please complete the following information):

• OS: [Windows 10]
• Compiler: [MSVC2019]
• PCL Version [last]

Possible Solution
very generic and not much helpful hint:
give some warnings at some level, either compile or run time

The code using it

    if (globalState.calcNormal.get())
    {
        bool openMP = globalState.openMP.get();
        pcl::NormalEstimation<PointXYZRGB, Normal> n; 
        pcl::NormalEstimationOMP<PointXYZRGB, Normal> nomp(4); // instantiate n threads
        nomp.setNumberOfThreads(4);
        PointCloud<Normal>::Ptr normals(new PointCloud<Normal>());
        auto begin = std::chrono::high_resolution_clock::now();
        if (!openMP)
        {
            n.setInputCloud(filteredCloud);
            n.setKSearch(20);
            n.compute(*normals);
        }
        else
        {
            printf("\n using openMP\n");
            nomp.setInputCloud(filteredCloud);
            nomp.setKSearch(20);
            nomp.compute(*normals);
        }

        auto end = std::chrono::high_resolution_clock::now();
        auto elapsed = std::chrono::duration_cast<std::chrono::nanoseconds>(end - begin);
        printf("\n -----NORMALS TIME: %.3f msec.\n", elapsed.count() * 1e-6);

    }

[mynickmynick]

I am seeing now that _OPENMP is not #defined

[mynickmynick]

something like convolve.cpp should be also in normals

#ifndef _OPENMP
if (nb_threads > 1) {
pcl::console::print_info("OpenMP not activated. Number of threads: 1\n");

[mynickmynick]

i think the reason is that my MSVC was missing an intel integration, my fault, I had honestly forgot the need

but a warning of some kind I think should be generated somewhere by PCL

[mynickmynick]

also after the intel integration, still not working

i read suggested this for cmake?
target_link_libraries(MyTarget PUBLIC OpenMP::OpenMP_CXX)

[mvieth]

i read suggested this for cmake?
target_link_libraries(MyTarget PUBLIC OpenMP::OpenMP_CXX)

See here:

pcl/cmake/pcl_targets.cmake

Lines 200 to 202 in cc4bde5

	if(TARGET OpenMP::OpenMP_CXX)
	target_link_libraries(${_name} OpenMP::OpenMP_CXX)
	endif()

You could add the following to your CMake diagnostics:

  if(TARGET OpenMP::OpenMP_CXX)
    message(STATUS "OpenMP::OpenMP_CXX is available")
  else()
    message(STATUS "OpenMP::OpenMP_CXX is not available!")
  endif()

but a warning of some kind I think should be generated somewhere by PCL
something like convolve.cpp should be also in normals

I agree, I will look into it

I have no experience with using OpenMP with MSVC, so I can't help you with that problem.

[mynickmynick]

now it seems to work but I had to add in my root CmakeLists.txt

find_package(OpenMP COMPONENTS C CXX)
if(OpenMP_FOUND)
string(APPEND CMAKE_C_FLAGS " ${OpenMP_C_FLAGS}")
string(APPEND CMAKE_CXX_FLAGS " ${OpenMP_CXX_FLAGS}")

endif()

the target_link_libraries(${RootProjectName} OpenMP::OpenMP_CXX)
instead seems not necessary in this CmakeLists

I am sorry I thought it was sufficient the linking inside the PCL to openMP
instead I have to explicitely link also from my root CmakeLists

is that "normal"? (meaning "ordinary" not "normal to the cloud" ;)) )

thanks

[larshg]

I have tried to reproduce it, but to me it seems to work well with multithreading, even though I haven't added OpenMP to my descendent project.
Now you mention root CMakelists - does that mean you include PCL as a submodule and include PCLs CMakelist in yours?

[larshg]

The defines might not show up correctly in a descendent opened project, since its not defined in that project. But it should still call and run OpenMP accelerated methods, if PCL was build with OpenMP support.

[mynickmynick]

I don't know if I understood well your question but I will try to better explain.
I am not an expert of Cmake so I might have made something wrong
What I have is a CMake project which is an application that links to PCL as well as other libraries like openCV , RealSense , ImGUI and other stuff.
In the CMakeLists.txt of the main directory of the project I have

find_package(PCL 1.2 REQUIRED)
actually I have also another probably redundant
find_package(PCL 1.2 REQUIRED)
in another CMakelIsts.txt down the folder submodules tree (Devices/Realsense/CmakeLists.txt)

then I have
Devices/Realsense/CmakeLists.txt
include_directories(${PCL_INCLUDE_DIRS})
link_directories(${PCL_LIBRARY_DIRS})
add_definitions(${PCL_DEFINITIONS})
target_link_libraries (RealSense ${PCL_LIBRARIES})

myImGUI/CmakeLists.txt
include_directories(${PCL_INCLUDE_DIRS})
link_directories(${PCL_LIBRARY_DIRS})
add_definitions(${PCL_DEFINITIONS})
target_link_libraries (myImGUI ${PCL_LIBRARIES})

and in a Globals.h I have

#define PCL_NO_PRECOMPILE //this define is very important!! otherwise many commands of the library do not work with XYZRGB points

I hope this is all, I am not finding other configurations

Actually as I wrote i do target_link_libraries ( ${PCL_LIBRARIES}) only inside 2 submodules, not in the main CMakeLists.txt

Inside the main CMakeLists.txt I have

target_link_libraries(${RootProjectName} myImGUI Devices)

In the main CMakeLists.txt I had to add

find_package(OpenMP COMPONENTS C CXX)
if(OpenMP_FOUND)
string(APPEND CMAKE_C_FLAGS " ${OpenMP_C_FLAGS}")
string(APPEND CMAKE_CXX_FLAGS " ${OpenMP_CXX_FLAGS}")
message(STATUS "Found OpenMP, spec date ${OpenMP_CXX_SPEC_DATE}")
endif()

[mynickmynick]

Now you mention root CMakelists - does that mean you include PCL as a submodule and include PCLs CMakelist in yours?

I only link to PCL with target_link_libraries

The defines might not show up correctly in a descendent opened project, since its not defined in that project. But it should still call and run OpenMP accelerated methods, if PCL was build with OpenMP support.

PCL was build with openMP support (WITH_OPENMP=true) but nope the number of threads was 1

1> Command line: "C:\Windows\system32\cmd.exe" /c "%SYSTEMROOT%\System32\chcp.com 65001 >NUL && "C:\PROGRAM FILES (X86)\MICROSOFT VISUAL STUDIO\2019\COMMUNITY\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\CMake\bin\cmake.exe" -G "Ninja" -DCMAKE_BUILD_TYPE:STRING="Release" -DCMAKE_INSTALL_PREFIX:PATH="D:\U\Cdev\C++\repo\external\forks\pcl\out\install\x64-Release" -DBUILD_CUDA:BOOL="False" -DCMAKE_BUILD_TYPE:STRING="Release" -DVTK_DIR:PATH="C:/Users/User/source/repos/VTK/out/install/x64-Release" -DBUILD_visualization:BOOL="True" -DWITH_OPENMP:BOOL="True" -DBUILD_global_tests:BOOL="True" -DQhull_DIR:PATH="D:/U/Cdev/C++/repo/external/forks/qhull/out/install/x64-Release" -DCMAKE_CUDA_STANDARD:STRING="14" -DCMAKE_CXX_STANDARD:STRING="14" -DWITH_CUDA:BOOL="False" -DCMAKE_MAKE_PROGRAM="C:\PROGRAM FILES (X86)\MICROSOFT VISUAL STUDIO\2019\COMMUNITY\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\Ninja\ninja.exe" -DCMAKE_TOOLCHAIN_FILE="C:/Users/User/vcpkg/vcpkg/scripts/buildsystems/vcpkg.cmake" "D:\U\Cdev\C++\repo\external\forks\pcl" 2>&1"

1> [CMake] -- Found OpenMP, spec date 200203

1> [CMake] -- PCL build with following flags:
1> [CMake] -- /DWIN32 /D_WINDOWS /W3 /GR /EHsc /bigobj /FS -openmp

[larshg]

I recently created this #5718
Can you update PCL and see if it makes a difference?
I can also try a commit earlier without this fix, but I'm quite sure I did that, when I tested it.

But I noticed the AVX flags wasn't applied correctly during the multithread test and got a memory error when releasing a pointcloud.

The openmp part of that code, tries to detect the number of cores and supply that value to set how many theads it can use.
It could have had a impact, but I think it default to 0, which makes openMP check at runtime AFAIK.

[larshg]

I recently created this #5718 Can you update PCL and see if it makes a difference? I can also try a commit earlier without this fix, but I'm quite sure I did that, when I tested it.

But I noticed the AVX flags wasn't applied correctly during the multithread test and got a memory error when releasing a pointcloud.

The openmp part of that code, tries to detect the number of cores and supply that value to set how many theads it can use. It could have had a impact, but I think it default to 0, which makes openMP check at runtime AFAIK.

Nm this. The mp count in this case is number if threads when compiling 😳

[larshg]

Could you set a breakpoint in the omp for loop and see the thread window to see if multiple threads get spawned, when you step forward. A blue circle will also appear at the program pointer, whenever it does a thread shift.