pybind integration breaks CMake

[andreistoian]

For pybind there was a change in the way libraries are linked to the cuda target: the keyword PUBLIC was added to the target_link_libraries.

CMake (at least my version: 3.5.1) does not like this as the FindCUDA.cmake default script does not specify the PUBLIC keyword when linking system CUDA libs (/usr/share/cmake-3.5/Modules/FindCUDA.cmake: 1660). It seems Cmake does not like if some libraries are declared PUBLIC and some not.

Workaround:

• Removing PUBLIC in the N2D2 cmakelists.txt file for cuda fixes this
• disable CUDA if not needed

[olivierbichler-cea]

This is a known issue and I though I had the solution using set(CUDA_LINK_LIBRARIES_KEYWORD PUBLIC) in the CMakeList.txt. And it works with the automated builds. So that's strange... did you clear everything before rebuilding? What is your version of CUDA?

[andreistoian]

It seems CUDA_LINK_LIBRARIES_KEYWORD was introduced in Cmake 3.9 but minimum cmake version is set to 3.1.

So we could either set minimum CMake version higher (but it would require installing a custom cmake on Ubuntu 16 for example) or maybe hack it with:

+    if(${CMAKE_VERSION} VERSION_LESS "3.9.0")
+        set(CUDA_N2D2_LINK_KEYWORD "")
+    else()
+        set(CUDA_LINK_LIBRARIES_KEYWORD PUBLIC)
+        set(CUDA_N2D2_LINK_KEYWORD PUBLIC)
+    endif()
+
     find_package(CuDNN REQUIRED)

     file(GLOB_RECURSE src_files_cuda "src/*.cu")
@@ -217,11 +223,11 @@ if(CUDA_FOUND)

     # CUDA
     target_include_directories(n2d2_lib_cuda PUBLIC ${CUDA_INCLUDE_DIRS})
-    target_link_libraries(n2d2_lib_cuda PUBLIC ${CUDA_LIBRARIES})
+    target_link_libraries(n2d2_lib_cuda ${CUDA_N2D2_LINK_KEYWORD} ${CUDA_LIBRARIES})

     # CuDNN
     target_include_directories(n2d2_lib_cuda PUBLIC ${CUDNN_INCLUDE_DIRS})
-    target_link_libraries(n2d2_lib_cuda PUBLIC ${CUDNN_LIBRARY})
+    target_link_libraries(n2d2_lib_cuda ${CUDA_N2D2_LINK_KEYWORD} ${CUDNN_LIBRARY})

     # Cublas
     # Work-around due to a bug in CMake < 3.12.2
@@ -229,11 +235,11 @@ if(CUDA_FOUND)
          list(REMOVE_ITEM CUDA_CUBLAS_LIBRARIES "CUDA_cublas_device_LIBRARY-NOTFOUND")
     endif()

-    target_link_libraries(n2d2_lib_cuda PUBLIC ${CUDA_CUBLAS_LIBRARIES})
+    target_link_libraries(n2d2_lib_cuda ${CUDA_N2D2_LINK_KEYWORD} ${CUDA_CUBLAS_LIBRARIES})

But I guess the PUBLIC keyword was useful for pybind? Would it pose a problem to link without PUBLIC?

[olivierbichler-cea]

Yes, the issue is that the pybind11_add_module command uses link keyword for target_link_libraries internally and CMake don't allow to mix target_link_libraries with and without keywords... It would be preferable to manage to make a workaround for CUDA for older versions too, because keywords is now the recommended style for using target_link_libraries.

[olivierbichler-cea]

Perhaps the solution is to include our own FindCUDA, like this one: https://github.com/CLIUtils/cuda_support This may be a viable solution for older versions of CMake because FindCUDA is deprecated anyway...

[olivierbichler-cea]

This should be fixed in the latest commit, could you try again?

[andreistoian]

Sorry, it doesn't work since the FindCUDA that you added needs other features that CMake 3.5 does not have:

  Unknown CMake command "cmake_initialize_per_config_variable".

I think it's best to just require Cmake 3.10. On ubuntu 16 it can easily be installed following the instructions from [https://apt.kitware.com/]

I upgraded to cmake 3.19. This works for CUDA. However, it causes some other problem with pybind (for python2.7). I'm getting this message on a new cmake build:

CMake Error at /usr/local/lib/python2.7/dist-packages/pybind11/share/cmake/pybind11/pybind11NewTools.cmake:211 (if):
  if given arguments:

    "MODULE" "STREQUAL" "MODULE" "AND" "(" "NOT" "ARG_WITHOUT_SOABI" "OR" "NOT" "WITH_SOABI" "IN_LIST" "ARG_UNPARSED_ARGUMENTS" ")"

  Unknown arguments specified

Are you using python 2.7 our 3.5? the python version does not seem to be configurable in the CMakelists.txt file.

[olivierbichler-cea]

Ok, unfortunately I could not test this correctly... but actually I am a bit surprised because the whole thing works on Travis on Ubuntu 14.04, which is even older. Could you try by just removing the offending line in the FindCUDA.cmake file? I think it appears only once in the file and I don't think it is necessary in our case. Regarding the second error, I think this is an issue between your version of CMake and the default Python PyBind package. I don't think the Python version matter here, even though we only test on Python 3.x.

[e-dupuis]

I ran into the same issue while trying to use the latest version of N2D2 in a docker using this Dockerfile:

FROM cealist/n2d2

RUN apt purge --auto-remove cmake -y && \
    apt update && apt install libssl-dev -y && \
    version=3.19 && \
    build=1 && \
    mkdir ~/temp && \
    cd ~/temp &&\
    wget https://cmake.org/files/v$version/cmake-$version.$build.tar.gz && \
    tar -xzvf cmake-$version.$build.tar.gz && \
    cd cmake-$version.$build/ && \
    ./bootstrap && \ 
    make -j$(nproc) && \
    make install && \
    cmake --version 

ENV PATH="/root/miniconda3/bin:${PATH}"
ARG PATH="/root/miniconda3/bin:${PATH}"
RUN apt-get update

RUN apt-get install -y wget && rm -rf /var/lib/apt/lists/*

RUN wget \
    https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh \
    && mkdir /root/.conda \
    && bash Miniconda3-latest-Linux-x86_64.sh -b \
    && rm -f Miniconda3-latest-Linux-x86_64.sh 
RUN conda --version

RUN apt update && apt install protobuf-compiler libprotoc-dev python3 python3-dev -y
RUN conda install pybind11 -c conda-forge

WORKDIR $N2D2_ROOT

RUN cmake --version
RUN git pull --rebase && cd build && \
    cmake .. && \
    make -j"$(nproc)"

I tried with latest cmake (3.19) and python3

[olivierbichler-cea]

Did you try with an older version of CMake? This seems like a CMake/Pybind related issue, not N2D2...

[e-dupuis]

Thanks for your answer, and it's true it is not related to N2D2, changing os version to ubuntu 18.04 solved the issue, I managed to build a working container with this Dockerfile (based on the one in this repo):

FROM nvidia/cuda:10.1-cudnn7-devel-ubuntu18.04

ENV TZ=Europe/Paris
RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone

RUN apt-get update && apt-get install -y \
        build-essential \
        cmake \
        git \
        wget \
        gnuplot \
        libopencv-dev \        
    protobuf-compiler \
        libprotoc-dev \
    python3-dev \
    python3 \
    python3-pip \
    python

RUN cmake --version

ENV N2D2_ROOT=/opt/N2D2
WORKDIR $N2D2_ROOT

RUN git clone https://github.com/CEA-LIST/N2D2.git . && \
    git submodule init && \
    git submodule update && \
    mkdir build && cd build && \
    cmake .. && \
    make -j"$(nproc)"

ENV N2D2_MODELS $N2D2_ROOT/models
ENV PATH $N2D2_ROOT/build/bin:$PATH

WORKDIR /workspace

[olivierbichler-cea]

Hi, The FindCUDA issue on CMake 3.5 should now be fixed in the latest commit. Regarding the PyBind issue, I recommend using the latest integrated PyBind11 sub-module in case of problem instead of the system library. Closing the issue for now. Please feel free to re-open it if necessary.