aboria / Aboria

Enables computations over a set of particles in N-dimensional space
https://aboria.github.io/Aboria
Other
105 stars 30 forks source link

Aboria with OpenMP Issue #29

Closed JabirSS closed 6 years ago

JabirSS commented 6 years ago

Dear Dr. Robins,

First of all thank you for the impressive work on Aboria. I was really excited when I first found out about it.

I have just started trying to use Aboria and so far I've managed to run the SPH example and play around a little. I have been trying to run Aboria with OpenMP on a workstation without a CUDA GPU and I have done the following:

1-Since I don't have CUDA I had to comment out the #include line in CudaInclude.h header file as the compiler was looking for it. 2-I also added set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fopenmp -lgomp -I /usr/include/thrust") to CMakeLists.txt in order to run Thrust without CUDA.

When I try to compile, Ninja throws off the following error: c++: error: LINK_LIBRARIES: No such file or directory c++: error: =: No such file or directory ninja: build stopped: subcommand failed.

I am using the Eclipse Photon IDE with cmake 3.5.1 and gcc 5.4.0 on Ubuntu 16.04.9

I assume there is something wrong with my CMakeLists file, but I have tried to pinpoint the issue to no avail. The code compiles just fine when I remove the OpenMP and Thrust files from the CMakeLists file.

Below is a copy of the CMakeLists file and I hope it is a silly quick fix. I'd greatly appreciate your help with this!

Thanks and regards,

Jabir -------------------------CMakeLists.txt file below------------------------- cmake_minimum_required(VERSION 2.8)

set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/Aboria/cmake" ${CMAKE_MODULE_PATH}) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++14") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fopenmp -lgomp -I /usr/include/thrust")

add_definitions(-fopenmp -lgomp -I /usr/include/thrust)

Boost

find_package(Boost 1.50.0 REQUIRED serialization) list(APPEND LIBRARIES ${Boost_LIBRARIES}) list(APPEND INCLUDES ${Boost_INCLUDE_DIRS})

VTK

find_package(VTK REQUIRED) if (VTK_FOUND) add_definitions(-DHAVE_VTK) endif(VTK_FOUND) list(APPEND LIBRARIES ${VTK_LIBRARIES}) list(APPEND INCLUDES ${VTK_INCLUDE_DIRS})

OpenMP

find_package(OpenMP REQUIRED) add_definitions(-DHAVE_OPENMP) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}") set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")

Thrust

find_package(Thrust REQUIRED) add_definitions(-DHAVE_THRUST)

Aboria

set(Aboria_LOG_LEVEL 1 CACHE STRING "Logging level (1 = least, 3 = most)") add_definitions(-DABORIA_LOG_LEVEL=${Aboria_LOG_LEVEL}) list(APPEND INCLUDES Aboria/src) list(APPEND INCLUDES Aboria/third-party)

include_directories(src ${INCLUDES})

set(SOURCE getting_started.cpp )

add_executable(getting_started ${SOURCE}) target_link_libraries(getting_started ${LIBRARIES})

martinjrobins commented 6 years ago

Hi @JabirSS, thanks for trying out Aboria. You shouldn't need to do steps (1) and (2) you have detailed above. When you install Thrust, you should get all the required headers, and you don't need a GPU as Aboria can just use Thrust's openmp back-end. And the correct openmp compiler flags will be added by the openmp section in your CMakeLists.txt.

try adding these lines to your CMakeLists.txt after the add_definitions(-DHAVE_THRUST) bit:

list(APPEND LIBRARIES ${CUDA_LIBRARIES})
list(APPEND INCLUDES ${THRUST_INCLUDE_DIR})

It looks like you haven't given the thrust include dir to the compiler.

JabirSS commented 6 years ago

Dear Dr. Robinson,

Thank you for your swift reply.

I did as you have instructed but unfortunately the code still wouldn't compile. Interestingly though, it worked after I commented out the following set command in the CMakeLists:

set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")

martinjrobins commented 6 years ago

Ok, that is strange, this line looks fairly innocent The attached CMakeLists.txt works fine for me. Is this the same as your file?

CMakeLists.txt

JabirSS commented 6 years ago

I basically get the error if I include the following line:

set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${OpenMP_EXE_LINKER_FLAGS}")

That's the only difference!

martinjrobins commented 6 years ago

perhaps your FindOpenMP.cmake file is putting something in OpenMP_EXE_LINKER_FLAGS that it shouldn't (like LINK_LIBRARIES)? have a look at what is in OpenMP_EXE_LINKER_FLAGS and see. There should only be the required linker flags to enable openmp on your system (i.e. -fopenmp)

JabirSS commented 6 years ago

I had a look at my findOpenMP.cmake file but it didn't have any reference to OpenMP_EXE_LINKER_FLAGS. The "-fopenmp" flag is added however and the code runs in parallel just fine! I guess this might have to do with my version of CMake? I'm using CMake v. 3.5.1.

Thank you for the support! :)

martinjrobins commented 6 years ago

actually, looking at my own FindOpenMP.cmake it looks like I don't have this variable either. Perhaps this is only needed for older versions of cmake? In any case, I'll remove it from Aboria's CMakeLists.txt, and the documentation as well, since your cmake has an issue with it

Thank you for reporting the issue :)

martinjrobins commented 6 years ago

fixed in 8298930