Example to build custom application and link to libcudf #7671

isVoid · 2021-03-22T20:22:55Z

closes #4824, also closes #2547

This PR introduces a basic libcudf example to demonstrate building custom application and link to libcudf using CMake. Libcudf examples now lives under cpp/examples folder. There's a examples/build.sh that manages building and linking examples. Besides, this PR adds examples into gpuCI tests to make sure examples still builds as the main package evolves.

harrism · 2021-03-22T21:55:28Z

I don't think we should skip CI. I think we should have simple examples like this inside of the libcudf repo and we should CI them so that we catch breakages.

harrism

Thanks for creating this. Mostly just minor style / simplification suggestions.

cpp/example/CMakeLists.txt

cpp/example/Dockerfile

cpp/example/README.md

cpp/example/src/process_csv.cpp

harrism · 2021-03-22T22:27:53Z

One other thing: I think you should prepare for future libcudf examples, this is not likely to be the only one. So instead of putting it in /cpp/example, call the top level directory examples, and make a subdirectory for this example, e.g. /cpp/examples/basic.

Co-authored-by: Mark Harris <[email protected]>

codecov · 2021-03-24T02:57:26Z

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.08@d1deca8). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-21.08    #7671   +/-   ##
===============================================
  Coverage                ?   82.97%           
===============================================
  Files                   ?      109           
  Lines                   ?    18214           
  Branches                ?        0           
===============================================
  Hits                    ?    15113           
  Misses                  ?     3101           
  Partials                ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1deca8...41a6e3f. Read the comment docs.

harrism · 2021-03-24T03:00:07Z

@ajschmidt8 already approved for ops, but IMO we should get CI working on this example before it is merged...

Co-authored-by: Mark Harris <[email protected]>

isVoid · 2021-03-24T08:44:40Z

A few updates:

Code is more concise, removed helper function get_columns_from_table
Removed memory pool init
Removed ccache

harrism

Nice example. Thanks.

ttnghia · 2021-05-26T22:44:01Z

cpp/examples/build.sh

+# Add libcudf examples build scripts down below
+
+# Parallelism control
+PARALLEL_LEVEL=${PARALLEL_LEVEL:-4}


I'm not an expert in cmake/shell thus I'm not sure if this will result in -j 4 or -j -4?

Seems to be the same as here:

cudf/ci/gpu/build.sh

Line 17 in 0b9f178

export PARALLEL_LEVEL=${PARALLEL_LEVEL:-4}

I got it! We will have 4, not -4 so everything will be fine. The shell syntax is a bit confusing:

${parameter:-word} If parameter is unset or null, the expansion of word is substituted. Otherwise, the value of parameter is substituted.

Ref: https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html

harrism · 2021-05-26T22:52:24Z

@gpucibot merge

harrism · 2021-05-26T22:52:51Z

rerun tests

isVoid · 2021-05-26T23:26:39Z

@harrism should this rebase to 21.08? (rebasing since I see it on 21.08 board)

harrism · 2021-05-27T00:48:44Z

Oh, OK. I thought it was fine to go in 21.06.

galipremsagar · 2021-06-04T00:25:43Z

rerun tests

davidwendt · 2021-06-04T00:32:40Z

Would this also close issue #2547 ?

isVoid · 2021-06-07T22:39:14Z

rerun tests

isVoid · 2021-06-08T17:55:33Z

Looks like the failed CI builds are due to mis-configuration of paths in build.sh. Investigating.

cpp/examples/basic/CMakeLists.txt

…build-libcudf-app

isVoid · 2021-06-17T23:04:48Z

Thanks to @robertmaynard pointing out, that the examples should not build from main, because older examples may break with latest releases. Instead, they should explicitly link to the latest version, this way to which version the examples should work with is clear.

This requires auto bumping the version from update-version.sh, which is updated accordingly.

Requires re-review from @rapidsai/ops-codeowners

isVoid · 2021-06-18T01:27:06Z

Oops. I forgot to block merging. @robertmaynard can you still look at the CMake file and raise an issue for it?

robertmaynard · 2021-06-18T14:17:13Z

In my mind examples should have as little 'noise' so that readers can easily find
the important bits. To this end we would want to do the following:

Refactor the add_executable and target_link_libaries lines to look like:

add_executable(basic_example src/process_csv.cpp)
target_link_libraries(basic_example PRIVATE cudf::cudf)

This has multiple improvements:

We use real names instead of variables, making it easier to read
We don't quote relative paths, since that is uneeded
We use the Modern CMake pattern of marking target_link_libraries libraries
as public/private/interface.

Drop the set(CMAKE_CUDA_ARCHITECTURES "") line. The can be specified in the
build.sh. If we are wanting a nicer API for specifying which cuda archs we are
building for we could use rapids_cmake

Drop the set(CMAKE_EXPORT_COMPILE_COMMANDS ON) entirely. This is only needed for
some autocomplete tools.

Drop the set(CMAKE_CXX_STANDARD 17) line and instead at the end of the file do:

target_compile_features(basic_example PRIVATE cxx_std_17)

The project(basic_example is enabling the CUDA and C languages which aren't
needed to use cudf so we should drop them. If cudf is subsequently built
from source via CPM it will enable the CUDA language when needed

I think we can clean up the logic around downloading and using CPM, it currently has some logic that really only makes sense for 'production' projects. Plus it is using an old CPM version fetched from the old project URL.

I think we can simplify the cpm logic to be:

set(CPM_DOWNLOAD_VERSION v0.32.2)
file(DOWNLOAD https://github.com/cpm-cmake/CPM.cmake/releases/download/${CPM_DOWNLOAD_VERSION}/get_cpm.cmake ${CMAKE_BINARY_DIR}/cmake/get_cpm.cmake)
include(${CMAKE_BINARY_DIR}/cmake/get_cpm.cmake)

set(CUDF_VERSION 21.08)
CPMFindPackage(NAME  cudf
    GIT_REPOSITORY  https://github.com/rapidsai/cudf
    GIT_TAG         branch-${VERSION}
    GIT_SHALLOW     TRUE
    SOURCE_SUBDIR   cpp
)

Total end result would be the CMakeLists.txt looking like:

cmake_minimum_required(VERSION 3.18)

project(basic_example VERSION 0.0.1 LANGUAGES CXX)

set(CPM_DOWNLOAD_VERSION v0.32.2)
file(DOWNLOAD https://github.com/cpm-cmake/CPM.cmake/releases/download/${CPM_DOWNLOAD_VERSION}/get_cpm.cmake ${CMAKE_BINARY_DIR}/cmake/get_cpm.cmake)
include(${CMAKE_BINARY_DIR}/cmake/get_cpm.cmake)

set(CUDF_TAG branch-21.08)
CPMFindPackage(NAME  cudf
    GIT_REPOSITORY  https://github.com/rapidsai/cudf
    GIT_TAG         ${CUDF_TAG}
    GIT_SHALLOW     TRUE
    SOURCE_SUBDIR   cpp
)

# Configure your project here
add_executable(basic_example src/process_csv.cpp)
target_link_libraries(basic_example PRIVATE cudf::cudf)
target_compile_features(basic_example PRIVATE cxx_std_17)

) Follow up of #7671 This PR addresses review comments from #7671 (comment), modernizing the CMakeFile in libcudf basic example. This PR also updates build tests for examples to make sure they are tested after both regular/project flash code path. Authors: - Michael Wang (https://github.com/isVoid) Approvers: - Robert Maynard (https://github.com/robertmaynard) - Ray Douglass (https://github.com/raydouglass) - Mark Harris (https://github.com/harrism) URL: #8568

Migrating from link_to_libcudf repo

c128957

isVoid added libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue container 5 - DO NOT MERGE Hold off on merging; see PR for details labels Mar 22, 2021

isVoid requested review from a team as code owners March 22, 2021 20:22

isVoid requested review from trxcllnt and jrhemstad March 22, 2021 20:22

isVoid changed the title ~~[skip-ci] Example to build custom application and link to libcudf~~ Example to build custom application and link to libcudf Mar 22, 2021

harrism requested changes Mar 22, 2021

View reviewed changes

randerzander requested a review from jdye64 March 23, 2021 04:21

ajschmidt8 approved these changes Mar 23, 2021

View reviewed changes

Doc, dockerfile revise

fa688f1

Co-authored-by: Mark Harris <[email protected]>

harrism added feature request New feature or request non-breaking Non-breaking change labels Mar 24, 2021

isVoid and others added 7 commits March 23, 2021 21:37

Multiple library usage, code style fixes.

7ef5fe0

Co-authored-by: Mark Harris <[email protected]>

several code style updates after merge

abf20ab

further trim down codes

43ebd65

Just using basic rmm mr

eed062d

remove unused imports

22d8bcb

simpler cmakefile

a9a3bb5

removed ccmake dependency

d3b789c

isVoid removed the 5 - DO NOT MERGE Hold off on merging; see PR for details label Mar 24, 2021

harrism approved these changes May 25, 2021

View reviewed changes

harrism requested a review from ttnghia May 26, 2021 22:36

ttnghia approved these changes May 26, 2021

View reviewed changes

harrism added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels May 26, 2021

isVoid changed the base branch from branch-21.06 to branch-21.08 May 26, 2021 23:54

isVoid added 0 - Waiting on Author Waiting for author to respond to review and removed 5 - Ready to Merge Testing and reviews complete, ready to merge labels Jun 8, 2021

Abs path in build.sh

66dd82c

isVoid commented Jun 9, 2021

View reviewed changes

cpp/examples/basic/CMakeLists.txt Outdated Show resolved Hide resolved

cpp/examples/basic/CMakeLists.txt Outdated Show resolved Hide resolved

isVoid requested a review from robertmaynard June 17, 2021 20:57

isVoid added 2 commits June 17, 2021 14:22

Merge branch 'branch-21.08' of https://github.com/rapidsai/cudf into …

145a96e

…build-libcudf-app

Use latest branch and configure auto update

41a6e3f

rapids-bot bot merged commit adaca18 into rapidsai:branch-21.08 Jun 18, 2021

isVoid mentioned this pull request Jun 19, 2021

Modernize libcudf basic example CMakeFile; updates CI build tests #8568

Merged

charlesbluca mentioned this pull request Sep 24, 2021

[PROPOSAL] Always use pre-commit to run Python linters #9309

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Example to build custom application and link to libcudf #7671

Example to build custom application and link to libcudf #7671

isVoid commented Mar 22, 2021 •

edited

Loading

harrism commented Mar 22, 2021

harrism left a comment

harrism commented Mar 22, 2021

codecov bot commented Mar 24, 2021 •

edited

Loading

harrism commented Mar 24, 2021

isVoid commented Mar 24, 2021 •

edited

Loading

harrism left a comment

ttnghia May 26, 2021 •

edited

Loading

harrism May 26, 2021

ttnghia May 26, 2021 •

edited

Loading

harrism commented May 26, 2021

harrism commented May 26, 2021

isVoid commented May 26, 2021 •

edited

Loading

harrism commented May 27, 2021

galipremsagar commented Jun 4, 2021

davidwendt commented Jun 4, 2021

isVoid commented Jun 7, 2021

isVoid commented Jun 8, 2021

isVoid commented Jun 17, 2021 •

edited

Loading

isVoid commented Jun 18, 2021 •

edited

Loading

robertmaynard commented Jun 18, 2021 •

edited

Loading

Example to build custom application and link to libcudf #7671

Example to build custom application and link to libcudf #7671

Conversation

isVoid commented Mar 22, 2021 • edited Loading

harrism commented Mar 22, 2021

harrism left a comment

Choose a reason for hiding this comment

harrism commented Mar 22, 2021

codecov bot commented Mar 24, 2021 • edited Loading

Codecov Report

harrism commented Mar 24, 2021

isVoid commented Mar 24, 2021 • edited Loading

harrism left a comment

Choose a reason for hiding this comment

ttnghia May 26, 2021 • edited Loading

Choose a reason for hiding this comment

harrism May 26, 2021

Choose a reason for hiding this comment

ttnghia May 26, 2021 • edited Loading

Choose a reason for hiding this comment

harrism commented May 26, 2021

harrism commented May 26, 2021

isVoid commented May 26, 2021 • edited Loading

harrism commented May 27, 2021

galipremsagar commented Jun 4, 2021

davidwendt commented Jun 4, 2021

isVoid commented Jun 7, 2021

isVoid commented Jun 8, 2021

isVoid commented Jun 17, 2021 • edited Loading

isVoid commented Jun 18, 2021 • edited Loading

robertmaynard commented Jun 18, 2021 • edited Loading

isVoid commented Mar 22, 2021 •

edited

Loading

codecov bot commented Mar 24, 2021 •

edited

Loading

isVoid commented Mar 24, 2021 •

edited

Loading

ttnghia May 26, 2021 •

edited

Loading

ttnghia May 26, 2021 •

edited

Loading

isVoid commented May 26, 2021 •

edited

Loading

isVoid commented Jun 17, 2021 •

edited

Loading

isVoid commented Jun 18, 2021 •

edited

Loading

robertmaynard commented Jun 18, 2021 •

edited

Loading