Link libcudf_kafka against cuDF CMake export targets (CPM) #7484

jdye64 · 2021-03-02T16:22:21Z

Now that #7107 is merged libcudf_kafka needs to link against those newly exposed cudf::cudf and rmm::rmm targets otherwise the build fails.

cpp/libcudf_kafka/CMakeLists.txt

cpp/libcudf_kafka/cmake/thirdparty/CUDF_KAFKA_GetCUDF.cmake

cpp/libcudf_kafka/CMakeLists.txt

docs/cudf/source/conf.py

…not exist

cpp/libcudf_kafka/cmake/thirdparty/CUDF_KAFKA_GetCUDF.cmake

…_cpm_exports

jdye64 · 2021-03-05T22:36:44Z

rerun tests

…_cpm_exports

trxcllnt · 2021-03-19T00:39:05Z

Seems like CPM is still building libcudf, even though it's installed:

-- CPM: adding package [email protected] (branch-0.19)

Is this line why find_package(cudf) is failing to find libcudf?

-- Unable to find cuda_runtime.h in "/jenkins/workspace/rapidsai/gpuci/cudf/prb/cudf-cpu-cuda-build/CUDA/include" for CUDAToolkit_INCLUDE_DIR.

Unrelated, but should these all have -real after them? I thought just the last one should?

-- CUDF: Building CUDF for GPU architectures: 60-real;70-real;75-real;80

jdye64 · 2021-03-19T00:41:07Z

I’m glad you mentioned this. I was noticing this in my third party application builds the other day. Does it once but then doesn’t again. However in CI it’s always a fresh environment so always does it Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Paul Taylor ***@***.***> Sent: Thursday, March 18, 2021 8:39:24 PM To: rapidsai/cudf ***@***.***> Cc: Jeremy Dyer ***@***.***>; Mention ***@***.***> Subject: Re: [rapidsai/cudf] Link libcudf_kafka against cuDF CMake export targets (CPM) (#7484) Seems like CPM is still building libcudf, even though it's installed:

-- CPM: adding package ***@***.*** (branch-0.19) Unrelated, but should these all have -real after them? I thought just the last one should?

-- CUDF: Building CUDF for GPU architectures: 60-real;70-real;75-real;80 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#7484 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAQHLA4SVW2EJIRGN7WOW2LTEKMLZANCNFSM4YPJGA6A>.

trxcllnt · 2021-03-19T00:42:21Z

Yeah, I'm seeing this in CI for my cuSpatial PR as well. I just finished downloading the nightly image that should have cuDF in conda, will investigate why CPM can't seem to find libcudf.

robertmaynard · 2021-03-19T13:48:34Z

-- CUDF: Building CUDF for GPU architectures: 60-real;70-real;75-real;80

That is correct. -real means just generate for that architecture ( SASS ), while 80 means generate SASS and PTX.

@trxcllnt

This PR makes `find_package(cudf)` work when `libcudf` is installed via conda. This should make CPM use the conda-installed `libcudf` in #7484 and rapidsai/cuspatial#365. Tested in a `gpuci/miniconda-cuda:10.2-devel-ubuntu18.04` container. Install dependencies: ``` mamba install -n base \ -c conda-forge gtest gmock cmake=3.18 boost-cpp=1.72.0 \ -c rapidsai-nightly libcudf ``` Use this `CMakeLists.txt`: ```cmake cmake_minimum_required(VERSION 3.18) project(test VERSION 1.0.0 LANGUAGES C CXX CUDA) find_package(cudf) add_library(test test.cpp) target_link_libraries(test cudf::cudf) ``` <details><summary>See this output:</summary><pre> (base) root@8c350dfb37f1:/tmp/findpackagecudf# mkdir build && cmake -B build -S . -- The C compiler identification is GNU 7.5.0 -- The CXX compiler identification is GNU 7.5.0 -- The CUDA compiler identification is NVIDIA 10.2.89 -- 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 -- Detecting CUDA compiler ABI info -- Detecting CUDA compiler ABI info - done -- Check for working CUDA compiler: /usr/local/cuda/bin/nvcc - skipped -- Detecting CUDA compile features -- Detecting CUDA compile features - done -- Found CUDAToolkit: /usr/local/cuda/include (found version "10.2.89") -- 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 ZLIB: /opt/conda/lib/libz.so (found version "1.2.11") -- Found Boost: /opt/conda/include (found suitable version "1.72.0", minimum required is "1.65.0") found components: filesystem -- Found Thrust: /usr/local/cuda/include (found suitable version "1.9.7", minimum required is "1.9.0") -- Found rmm: /opt/conda/lib/cmake/rmm/rmm-config.cmake (found suitable version "0.19.0", minimum required is "0.19") -- Found GTest: /opt/conda/lib/libgtest.so (Required is at least version "1.10.0") -- Found Thrust: /opt/conda/include/libcudf/Thrust/thrust/cmake/thrust-config.cmake (found suitable version "1.10.0.0", minimum required is "1.10.0") -- Found cudf: /opt/conda/lib/cmake/cudf/cudf-config.cmake (found version "0.19.0") -- Configuring done -- Generating done -- Build files have been written to: /tmp/findpackagecudf/build </pre></details> <details><summary>The generated compile command for test.cpp:</summary><pre> /usr/bin/c++ -DBOOST_NO_CXX14_CONSTEXPR -DCUDF_VERSION=0.19.0 -DJITIFY_USE_CACHE -DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_INFO -DTHRUST_DEVICE_SYSTEM=THRUST_DEVICE_SYSTEM_CUDA -DTHRUST_HOST_SYSTEM=THRUST_HOST_SYSTEM_CPP -I/opt/conda/include/libcudf/Thrust/dependencies/cub -isystem /opt/conda/include -isystem /opt/conda/include/libcudf/libcudacxx -isystem /opt/conda/include/libcudf/Thrust -isystem /usr/local/cuda/include -fPIC -o CMakeFiles/test.dir/test.cpp.o -c /tmp/findpackagecudf/test.cpp </pre></details> Authors: - Paul Taylor (@trxcllnt) Approvers: - Keith Kraus (@kkraus14) URL: #7658

kkraus14 · 2021-03-20T06:10:31Z

rerun tests

trxcllnt · 2021-03-20T19:53:31Z

rerun tests

jdye64 · 2021-03-22T09:40:27Z

hmm seeing some odd errors about RMM? Any ideas @trxcllnt ?

  Attempt to promote imported target "rmm::rmm" to global scope (by setting
  IMPORTED_GLOBAL) which is not built in this directory.
Call Stack (most recent call first):
  build/_deps/cudf-src/cpp/cmake/thirdparty/CUDF_GetRMM.cmake:52 (fix_cmake_global_defaults)
  build/_deps/cudf-src/cpp/cmake/thirdparty/CUDF_GetRMM.cmake:62 (find_and_configure_rmm)
  build/_deps/cudf-src/cpp/CMakeLists.txt:131 (include)```

robertmaynard · 2021-03-22T13:32:58Z

hmm seeing some odd errors about RMM? Any ideas @trxcllnt ?

  Attempt to promote imported target "rmm::rmm" to global scope (by setting
  IMPORTED_GLOBAL) which is not built in this directory.
Call Stack (most recent call first):
  build/_deps/cudf-src/cpp/cmake/thirdparty/CUDF_GetRMM.cmake:52 (fix_cmake_global_defaults)
  build/_deps/cudf-src/cpp/cmake/thirdparty/CUDF_GetRMM.cmake:62 (find_and_configure_rmm)
  build/_deps/cudf-src/cpp/CMakeLists.txt:131 (include)```

I wonder if the problem is that the rmm::rmm target exists before we start to include cudf, and we try to add it again and the GLOBAL import logic inside find_and_configure_rmm fails.

Might just want to early terminate find_and_configure_rmm if the rmm::rmm target already exists.

jdye64 · 2021-03-22T13:54:40Z

I’m not very familiar with that part of the code base. Care to show me what that snippet might look like and I could just add it to this PR and see what happens. If it works can just be part of this PR Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Robert Maynard ***@***.***> Sent: Monday, March 22, 2021 9:33:16 AM To: rapidsai/cudf ***@***.***> Cc: Jeremy Dyer ***@***.***>; Mention ***@***.***> Subject: Re: [rapidsai/cudf] Link libcudf_kafka against cuDF CMake export targets (CPM) (#7484) hmm seeing some odd errors about RMM? Any ideas @trxcllnt<https://github.com/trxcllnt> ? Attempt to promote imported target "rmm::rmm" to global scope (by setting IMPORTED_GLOBAL) which is not built in this directory. Call Stack (most recent call first): build/_deps/cudf-src/cpp/cmake/thirdparty/CUDF_GetRMM.cmake:52 (fix_cmake_global_defaults) build/_deps/cudf-src/cpp/cmake/thirdparty/CUDF_GetRMM.cmake:62 (find_and_configure_rmm) build/_deps/cudf-src/cpp/CMakeLists.txt:131 (include)``` I wonder if the problem is that the rmm::rmm target exists before we start to include cudf, and we try to add it again and the GLOBAL import logic inside find_and_configure_rmm fails. Might just want to early terminate find_and_configure_rmm if the rmm::rmm target already exists. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#7484 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAQHLA5SZNUQYQTBYDM7FYDTE5BJZANCNFSM4YPJGA6A>.

robertmaynard · 2021-03-22T13:59:54Z

I’m not very familiar with that part of the code base. Care to show me what that snippet might look like and I could just add it to this PR and see what happens. If it works can just be part of this PR

Try this change ( https://github.com/rapidsai/cudf/blob/branch-0.19/cpp/cmake/thirdparty/CUDF_GetRMM.cmake )

function(find_and_configure_rmm VERSION)
    if(TARGET rmm::rmm)
        return()
    endif

jdye64 · 2021-03-22T17:42:09Z

Ok, that fixed the RMM issue. Now I'm seeing this

Could NOT find cuFile (missing: cuFile_LIBRARY cuFileRDMA_LIBRARY)

any ideas there?

jdye64 · 2021-03-22T17:43:46Z

Ok, that fixed the RMM issue. Now I'm seeing this

Could NOT find cuFile (missing: cuFile_LIBRARY cuFileRDMA_LIBRARY)

any ideas there?

Oops, no, actually its still the RMM error. https://gpuci.gpuopenanalytics.com/job/rapidsai/job/gpuci/job/cudf/job/prb/job/cudf-cpu-cuda-build/CUDA=11.0/1720/consoleText

kkraus14 · 2021-03-22T17:56:37Z

-- CUDF: Building CUDF for GPU architectures: 52

Something else is off here as well.

robertmaynard · 2021-03-22T18:13:14Z

Ok, that fixed the RMM issue. Now I'm seeing this
Could NOT find cuFile (missing: cuFile_LIBRARY cuFileRDMA_LIBRARY)
any ideas there?

Oops, no, actually its still the RMM error. https://gpuci.gpuopenanalytics.com/job/rapidsai/job/gpuci/job/cudf/job/prb/job/cudf-cpu-cuda-build/CUDA=11.0/1720/consoleText

That failure looks to be coming from cudf to me:

-- CPM: adding package [email protected] (cudf_0.16)
-- CPM: adding package [email protected] (1.10.0)
-- CPM: using local package [email protected]
-- CPM: using local package [email protected]
-- CPM: adding package [email protected] (1.4.0)
-- CPM: using local package [email protected]
-- Could NOT find cuFile (missing: cuFile_LIBRARY cuFileRDMA_LIBRARY) 
-- Configuring done

Looks like it found the system installed rmm and failed a couple steps after that when looking cuFile.

kkraus14 · 2021-03-22T18:24:40Z

Looks like it found the system installed rmm and failed a couple steps after that when looking cuFile.

That's an optional dependency so it shouldn't be an issue.

I think the error is this:

CMake Error at build/_deps/cudf-src/cpp/cmake/thirdparty/CUDF_GetCPM.cmake:27 (set_target_properties):
  Attempt to promote imported target "rmm::rmm" to global scope (by setting
  IMPORTED_GLOBAL) which is not built in this directory.
Call Stack (most recent call first):
  build/_deps/cudf-src/cpp/cmake/thirdparty/CUDF_GetRMM.cmake:52 (fix_cmake_global_defaults)
  build/_deps/cudf-src/cpp/cmake/thirdparty/CUDF_GetRMM.cmake:62 (find_and_configure_rmm)
  build/_deps/cudf-src/cpp/CMakeLists.txt:131 (include)

robertmaynard · 2021-03-22T18:28:10Z

Looks like it found the system installed rmm and failed a couple steps after that when looking cuFile.

That's an optional dependency so it shouldn't be an issue.

I think the error is this:

CMake Error at build/_deps/cudf-src/cpp/cmake/thirdparty/CUDF_GetCPM.cmake:27 (set_target_properties):
  Attempt to promote imported target "rmm::rmm" to global scope (by setting
  IMPORTED_GLOBAL) which is not built in this directory.
Call Stack (most recent call first):
  build/_deps/cudf-src/cpp/cmake/thirdparty/CUDF_GetRMM.cmake:52 (fix_cmake_global_defaults)
  build/_deps/cudf-src/cpp/cmake/thirdparty/CUDF_GetRMM.cmake:62 (find_and_configure_rmm)
  build/_deps/cudf-src/cpp/CMakeLists.txt:131 (include)

Okay I see that error now, but I see that it is happening before compiler detection which is not expected.
What is causing this invocation of CMake? And with what parameters?

kkraus14 · 2021-03-22T18:29:35Z

It's hitting this section here: https://github.com/rapidsai/cudf/blob/branch-0.19/build.sh#L192-L208

kkraus14 · 2021-03-22T18:34:22Z

cpp/libcudf_kafka/CMakeLists.txt

+# * find CUDAToolkit package
+# * determine GPU architectures
+# * enable the CMake CUDA language
+# * set other CUDA compilation flags
+include(../cmake/Modules/ConfigureCUDA.cmake)


I think this is causing problems. There's no device code / actual cuda configuration needed for libcudf, and we're doing this before importing cudf / rmm / etc. which causes the CUDA language to be enabled which breaks things I think?

In my dummy CMakeLists.txt for #7658, CUDA had to be enabled for the find_package(cudf) to succeed.

Ok, that would make sense actually since I removed CUDA in a previous commit. Both @kkraus14 and I were both under the impression it was not needed since libcudf_kafka doesn't use CUDA explicitly. Are you saying I need to add it back?

My thought is that if someone needs to enable CUDA before doing find_package(cudf), we have something broken in our cudf cmake config.

It looks like cuftestutil and cudf both have some CUDA information in the export information. That would require downstream consumers to enable CUDA.

cuftestutil is tricky as it is a static library with CUDA sources, so CMake needs to export that when linking downstream it has CUDA dependencies ( so you need to enable the language )

cudf is leaking via compile flags, would could be fixed

Both of these should be resolved by having cudf-config.cmake have ( enable_language(CUDA) ) in the short term while we sort out the above two issues.

@robertmaynard

This integrates the changes from #7484 plus others required to get `cudf_kafka` to build cleanly after the CMake refactoring of `cudf` Authors: - Robert Maynard (@robertmaynard) - Jeremy Dyer (@jdye64) Approvers: - Mike Wendt (@mike-wendt) - Keith Kraus (@kkraus14) URL: #7674

robertmaynard · 2021-03-25T14:40:17Z

I think this can be closed now that #7674 has been merged.

jdye64 requested a review from a team as a code owner March 2, 2021 16:22

github-actions bot added CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. labels Mar 2, 2021

jdye64 marked this pull request as draft March 2, 2021 16:23

kkraus14 reviewed Mar 2, 2021

View reviewed changes

trxcllnt added non-breaking Non-breaking change 2 - In Progress Currently a work in progress code quality Java Affects Java cuDF API. improvement Improvement / enhancement to an existing function labels Mar 2, 2021

GPUtester and others added 5 commits March 2, 2021 12:06

REL v0.13.0 release

48edee7

enable cudf_kafka to use CPMFindPackage

54ceb3a

fixed copy/paste errors

9e7f3f9

enable cudf_kafka to use CPMFindPackage

25468f6

update libcudf_kafka version with each release

beaf86e

jdye64 force-pushed the link_against_cpm_exports branch from cdce3e2 to beaf86e Compare March 2, 2021 17:10

github-actions bot added gpuCI and removed Java Affects Java cuDF API. labels Mar 2, 2021

jdye64 commented Mar 2, 2021

View reviewed changes

docs/cudf/source/conf.py Outdated Show resolved Hide resolved

change version to 0.19 from 0.19.0 which leads to a branch that does …

464bcdd

…not exist

kkraus14 reviewed Mar 3, 2021

View reviewed changes

cpp/libcudf_kafka/cmake/thirdparty/CUDF_KAFKA_GetCUDF.cmake Show resolved Hide resolved

jdye64 added 5 commits March 3, 2021 15:18

checkpoint commit, does it build without tests in ci?

c3df905

updated to include ConfigureCUDA like cudf

089855c

update GPU Eval script location

c8ffeaa

Add Set/Get GPU eval cmake files

8fb7ae5

Merge remote-tracking branch 'upstream/branch-0.19' into link_against…

be8ea10

…_cpm_exports

jdye64 added 2 commits March 8, 2021 12:44

Merge remote-tracking branch 'upstream/branch-0.19' into link_against…

2a84c8f

…_cpm_exports

isort modified file

52a138b

updates from review

21a0502

trxcllnt mentioned this pull request Mar 19, 2021

Fix find_package(cudf) #7658

Merged

remove custom test target from build.sh file

bc3880f

jdye64 added 2 commits March 22, 2021 10:03

return if rmm::rmm target already exists

59855da

syntax issue

db4704c

kkraus14 reviewed Mar 22, 2021

View reviewed changes

robertmaynard mentioned this pull request Mar 24, 2021

cudf_kafka now uses cuDF CMake export targets (CPM) #7674

Merged

kkraus14 closed this Mar 25, 2021

jdye64 deleted the link_against_cpm_exports branch April 25, 2021 15:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Link libcudf_kafka against cuDF CMake export targets (CPM) #7484

Link libcudf_kafka against cuDF CMake export targets (CPM) #7484

jdye64 commented Mar 2, 2021

jdye64 commented Mar 5, 2021

trxcllnt commented Mar 19, 2021 •

edited

Loading

jdye64 commented Mar 19, 2021 via email

trxcllnt commented Mar 19, 2021

robertmaynard commented Mar 19, 2021

kkraus14 commented Mar 20, 2021

trxcllnt commented Mar 20, 2021

jdye64 commented Mar 22, 2021 •

edited

Loading

robertmaynard commented Mar 22, 2021

jdye64 commented Mar 22, 2021 via email

robertmaynard commented Mar 22, 2021

jdye64 commented Mar 22, 2021

jdye64 commented Mar 22, 2021

kkraus14 commented Mar 22, 2021

robertmaynard commented Mar 22, 2021

kkraus14 commented Mar 22, 2021

robertmaynard commented Mar 22, 2021

kkraus14 commented Mar 22, 2021

kkraus14 Mar 22, 2021

trxcllnt Mar 22, 2021

jdye64 Mar 22, 2021

kkraus14 Mar 22, 2021

robertmaynard Mar 22, 2021

robertmaynard Mar 22, 2021

robertmaynard commented Mar 25, 2021

Link libcudf_kafka against cuDF CMake export targets (CPM) #7484

Link libcudf_kafka against cuDF CMake export targets (CPM) #7484

Conversation

jdye64 commented Mar 2, 2021

jdye64 commented Mar 5, 2021

trxcllnt commented Mar 19, 2021 • edited Loading

jdye64 commented Mar 19, 2021 via email

trxcllnt commented Mar 19, 2021

robertmaynard commented Mar 19, 2021

kkraus14 commented Mar 20, 2021

trxcllnt commented Mar 20, 2021

jdye64 commented Mar 22, 2021 • edited Loading

robertmaynard commented Mar 22, 2021

jdye64 commented Mar 22, 2021 via email

robertmaynard commented Mar 22, 2021

jdye64 commented Mar 22, 2021

jdye64 commented Mar 22, 2021

kkraus14 commented Mar 22, 2021

robertmaynard commented Mar 22, 2021

kkraus14 commented Mar 22, 2021

robertmaynard commented Mar 22, 2021

kkraus14 commented Mar 22, 2021

kkraus14 Mar 22, 2021

Choose a reason for hiding this comment

trxcllnt Mar 22, 2021

Choose a reason for hiding this comment

jdye64 Mar 22, 2021

Choose a reason for hiding this comment

kkraus14 Mar 22, 2021

Choose a reason for hiding this comment

robertmaynard Mar 22, 2021

Choose a reason for hiding this comment

robertmaynard Mar 22, 2021

Choose a reason for hiding this comment

robertmaynard commented Mar 25, 2021

trxcllnt commented Mar 19, 2021 •

edited

Loading

jdye64 commented Mar 22, 2021 •

edited

Loading