alpaka-group / vikunja

Vikunja is a performance portable algorithm library that defines functions operating on ranges of elements for a variety of purposes . It supports the execution on multi-core CPUs and various GPUs. Vikunja uses alpaka to implement platform-independent primitives such as reduce or transform.
https://vikunja.readthedocs.io/en/latest/
Mozilla Public License 2.0
14 stars 5 forks source link

Modernize CMake #17

Closed SimeonEhrig closed 3 years ago

SimeonEhrig commented 3 years ago
SimeonEhrig commented 3 years ago

@j-stephan Can you please review it, because you added the CMake install instruction in alpaka

SimeonEhrig commented 3 years ago

/cc @sliwowitz

SimeonEhrig commented 3 years ago

@j-stephan @bernhardmgruber Can you please approve, if the PR is ready for merge

SimeonEhrig commented 3 years ago

@j-stephan @bernhardmgruber Can you please approve, if the PR is ready for merge

Please wait with approving. I found a horrible bug. I believe nesting alpaka is not possible. If I write

set(ALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLE ON)
find_package(vikunja)

or set ALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLE via argument, it is not pass throw to the find_package(alpaka) in the vikunja-config.cmake and the acc is not enabled. Any ideas? Otherwise I have to remove alpaka from vikunja, in the user have to handle alpaka manual.

bernhardmgruber commented 3 years ago

@j-stephan @bernhardmgruber Can you please approve, if the PR is ready for merge

Please wait with approving. I found a horrible bug. I believe nesting alpaka is not possible. ...

I let you solve that with @sbastrakov or @psychocoderHPC. I think I would just not set ALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLE myself but expect the user to set this on the cmake command line. But I do not mind how you solve that.

The rest is fine I think!

SimeonEhrig commented 3 years ago

@j-stephan @bernhardmgruber Can you please approve, if the PR is ready for merge

Please wait with approving. I found a horrible bug. I believe nesting alpaka is not possible. ...

I let you solve that with @sbastrakov or @psychocoderHPC. I think I would just not set ALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLE myself but expect the user to set this on the cmake command line. But I do not mind how you solve that.

The rest is fine I think!

In this case, the code was from my own example. Nothing for the public. In vikunja itself, the ALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLE is only enabled by default, if you build the examples or the tests. And if you install vikunja as library, it does not set any acc.

j-stephan commented 3 years ago

Just to make sure I understand this correctly: cmake -DALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLE=ON /path/to/vikunja does not enable the serial accelerator?

SimeonEhrig commented 3 years ago

Just to make sure I understand this correctly: cmake -DALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLE=ON /path/to/vikunja does not enable the serial accelerator?

Yes, if you mean /path/to/vinkunja is project, which uses vikunja via find_package(vikunja). But I found really interesting detail. At the cmake configure phase, there is the message, that the acc is enabled:

$ cmake -GNinja .. -DALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLE=ON
-- The C compiler identification is GNU 9.3.0
-- The CXX compiler identification is GNU 9.3.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Failed
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - found
-- Found Threads: TRUE  
-- Found Boost: /opt/spack-modules/linux-ubuntu20.04-skylake_avx512/gcc-10.2.0/boost-1.74.0-lqyr7ou7iafiwvq5fb6ipp62o6l2gqlf/lib/cmake/Boost-1.74.0/BoostConfig.cmake (found suitable version "1.74.0", minimum required is "1.65.1")  missing components: fiber
-- ALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLED
-- Configuring done
-- Generating done
-- Build files have been written to: /home/sehrig/programming/vikunja/rand/build
$ cmake --build .
[1/2] Building CXX object CMakeFiles/vikRand.dir/main.cpp.o
FAILED: CMakeFiles/vikRand.dir/main.cpp.o 
/usr/bin/c++ -DALPAKA_BLOCK_SHARED_DYN_MEMBER_ALLOC_KIB=30 -DALPAKA_DEBUG=0 -DALPAKA_DEBUG_OFFLOAD_ASSUME_HOST -DBOOST_ALL_NO_LIB -isystem /home/sehrig/application/miniconda3/envs/vikunja/include -Wall -Wextra -pedantic -MD -MT CMakeFiles/vikRand.dir/main.cpp.o -MF CMakeFiles/vikRand.dir/main.cpp.o.d -o CMakeFiles/vikRand.dir/main.cpp.o -c ../main.cpp
../main.cpp: In function ‘int main()’:
../main.cpp:11:29: error: ‘AccCpuSerial’ in namespace ‘alpaka::acc’ does not name a template type
   11 |   using TAcc = alpaka::acc::AccCpuSerial<alpaka::dim::DimInt<3u>, std::uint64_t>;
      |                             ^~~~~~~~~~~~
ninja: build stopped: subcommand failed.

I see, that the -DALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLED is missing in the compiler command.

Here are the CMakeLists.txt and main.cpp

cmake_minimum_required(VERSION 3.15)

set(_TARGET_NAME vikRand)
project(${_TARGET_NAME})

#find_package(alpaka REQUIRED)

find_package(vikunja REQUIRED)

alpaka_add_executable(${_TARGET_NAME})
target_sources(${_TARGET_NAME}
  PRIVATE
  main.cpp)
target_link_libraries(${_TARGET_NAME}
  PRIVATE
  #alpaka::alpaka
  vikunja::vikunja
  )
#include <alpaka/alpaka.hpp>
#include <vikunja/transform/transform.hpp>

#include <iostream>

int main(){

  using TAcc = alpaka::acc::AccCpuSerial<alpaka::dim::DimInt<3u>, std::uint64_t>;

  // data type of the data, which should be transformed
  using TData = float;
  //using Tix = alpaka::idx::Idx<TAcc>;
  //using Vec = alpaka::vec::Vec<TAcc>;

  std::cout << "Hello Vikunja" << std::endl;

  return 0;
}
psychocoderHPC commented 3 years ago
set(ALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLE ON)

You can not set options like it is a normal variable. set(ALPAKA_ACC_CPU_B_SEQ_T_SEQ_ENABLE ON) will not work. You must do it the ugly way: https://github.com/alpaka-group/cupla/blob/22dfdf4574d529b26523ce66368aaf2caea16566/cuplaConfig.cmake#L102-L110

SimeonEhrig commented 3 years ago

@j-stephan What do you think about the new target interalvikunja and the linkage on configure time?

j-stephan commented 3 years ago

Looks like a nice workaround for the problem. If this works with add_subdirectory, too, I'm okay with the merge.