Fix CMake files in libcudf C++ examples to use existing libcudf build if present #15348

mhaseeb123 · 2024-03-20T03:33:33Z

Description

This PR fixes the CMake artifacts for libcudf examples and includes CI updates to create executable libcudf-example conda package to run from CI

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.

copy-pr-bot · 2024-03-20T03:33:36Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

cpp/examples/build.sh

cpp/examples/fetch_dependencies.cmake

bdice

Great. This is getting closer to what I expect.

Do you think we should also run the binaries from libcudf-example in the C++ tests job in CI?

The steps to do that:

Add libcudf-example to

cudf/ci/test_cpp_common.sh

Lines 32 to 34 in 79d2dba

    
           rapids-mamba-retry install \ 
        
             --channel "${CPP_CHANNEL}" \ 
        
             libcudf libcudf_kafka libcudf-tests

Then add a section like this to the test_cpp.sh job that calls a new script called run_cudf_examples.sh. Track the error/exit code.

cudf/ci/test_cpp.sh

Lines 16 to 18 in 79d2dba

    
           rapids-logger "Run libcudf gtests" 
        
           ./ci/run_cudf_ctests.sh -j20 
        
           SUITEERROR=$?

Test that this fails in CI by pushing a change that compiles to an example binary but exits with a nonzero code.

cpp/examples/build.sh

cpp/examples/set_cuda_architecture.cmake

davidwendt · 2024-03-20T23:05:44Z

Do you think we should also run the binaries from libcudf-example in the C++ tests job in CI?

Just to be clear. This would just run the example to make sure it did not crash?
It would not actually verify the example produces the correct result, right?
I suppose there is value is checking for a crash only.
Perhaps it could run through compute-sanitizer.
This may be out of scope for this PR.

bdice · 2024-03-20T23:08:20Z

Yes, this is just to verify the example binaries (1) were built, (2) were packaged, and (3) exit 0 / do not crash. It sounds like we had issues with multiple aspects of the examples, not least of which was the build failing silently. The binaries could be run under sanitizers in the other memcheck test script but that is lower priority to me.

bdice · 2024-03-20T23:12:15Z

We could run the examples in CI in another PR but I don’t want to lose track of this. I’d prefer it in this PR since the changes should be relatively small and are somewhat related to the other build fixes here.

harrism · 2024-03-22T18:06:41Z

@mhaseeb123 would you mind making your PR title a little more specific? The title goes in CHANGELOG.md when we release so specific titles help make this file useful.

mhaseeb123 · 2024-03-22T20:16:39Z

@mhaseeb123 would you mind making your PR title a little more specific? The title goes in CHANGELOG.md when we release so specific titles help make this file useful.

Updated the PR title.

…23/cudf into cudf-examples-cmake-fix

mhaseeb123 · 2024-04-16T19:15:44Z

/ok to test

bdice

Nice. This is vastly improved. I have a few comments, but we're getting closer.

cpp/examples/basic/CMakeLists.txt

cpp/examples/build.sh

cpp/examples/fetch_dependencies.cmake

mhaseeb123 · 2024-04-17T03:58:38Z

/ok to test

mhaseeb123 · 2024-04-17T05:41:36Z

Weird CI build failure. Retriggering run.

cpp/examples/strings/CMakeLists.txt

mhaseeb123 · 2024-04-17T19:19:01Z

/ok to test

bdice

This seems fine to me. I think it'd be good to get approval from @robertmaynard / @KyleFromNVIDIA / @davidwendt since they all requested changes.

mhaseeb123 · 2024-04-18T00:31:27Z

/merge

All changes incorporated. Thank you!

mhaseeb123 · 2024-04-18T19:48:09Z

/merge

This PR adds a new example `parquet_io` to `libcudf/cpp/examples` instrumenting reading and writing parquet files with different column encodings (same for all columns for now) and compressions to close #15344. The example maybe elaborated and/or evolved as needed. #15348 should be merged before this PR to get all CMake updates needed to successfully build and run this example. Authors: - Muhammad Haseeb (https://github.com/mhaseeb123) Approvers: - Vukasin Milovanovic (https://github.com/vuule) - Ray Douglass (https://github.com/raydouglass) URL: #15420

updates for libcudf examples cmake files

6401ef2

mhaseeb123 requested review from a team as code owners March 20, 2024 03:33

mhaseeb123 requested review from vyasr and hyperbolic2346 March 20, 2024 03:33

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

mhaseeb123 added 3 - Ready for Review Ready for review by team improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 20, 2024

davidwendt previously requested changes Mar 20, 2024

View reviewed changes

cpp/examples/build.sh Outdated Show resolved Hide resolved

bdice reviewed Mar 20, 2024

View reviewed changes

cpp/examples/build.sh Outdated Show resolved Hide resolved

bdice reviewed Mar 20, 2024

View reviewed changes

cpp/examples/fetch_dependencies.cmake Outdated Show resolved Hide resolved

Applying suggested fixes

360d78a

bdice requested changes Mar 20, 2024

View reviewed changes

cpp/examples/build.sh Outdated Show resolved Hide resolved

cpp/examples/set_cuda_architecture.cmake Outdated Show resolved Hide resolved

mhaseeb123 changed the title ~~updates for libcudf examples cmake files~~ Fix examples CMake files to build with exisiting libcudf installation if present and automatically fetch CUDA_ARCHITECTURE Mar 22, 2024

mhaseeb123 changed the title ~~Fix examples CMake files to build with exisiting libcudf installation if present and automatically fetch CUDA_ARCHITECTURE~~ Fix examples CMake files to build with exisiting libcudf installation if present and fetch correct CUDA_ARCHITECTURE Mar 22, 2024

mhaseeb123 changed the title ~~Fix examples CMake files to build with exisiting libcudf installation if present and fetch correct CUDA_ARCHITECTURE~~ Fix examples CMake files to use exisiting libcudf build if present Mar 22, 2024

reverting CUDF_TAG to fetch_dependencies and handle in set_cuda_arch

24149bb

mhaseeb123 changed the title ~~Fix examples CMake files to use exisiting libcudf build if present~~ Fix CMake files in libcudf C++ examples to use exisiting libcudf build if present Mar 22, 2024

bdice changed the title ~~Fix CMake files in libcudf C++ examples to use exisiting libcudf build if present~~ Fix CMake files in libcudf C++ examples to use existing libcudf build if present Mar 22, 2024

Adding CI artifacts to build and run libcudf examples

90bd8b0

mhaseeb123 requested a review from a team as a code owner March 22, 2024 22:21

github-actions bot added the ci label Mar 22, 2024

mhaseeb123 and others added 4 commits April 16, 2024 19:10

incorporting suggested changes from reviews

ad3f6a6

Merge branch 'branch-24.06' into cudf-examples-cmake-fix

324e6dc

pre-commit eol

00d1e2b

Merge branch 'cudf-examples-cmake-fix' of https://github.com/mhaseeb1…

d767c49

…23/cudf into cudf-examples-cmake-fix

mhaseeb123 added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Apr 16, 2024

mhaseeb123 requested review from KyleFromNVIDIA, robertmaynard and bdice April 16, 2024 20:51

bdice reviewed Apr 17, 2024

View reviewed changes

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

cpp/examples/build.sh Outdated Show resolved Hide resolved

cpp/examples/fetch_dependencies.cmake Outdated Show resolved Hide resolved

mhaseeb123 and others added 3 commits April 17, 2024 03:54

apply reviewer suggestions

72089ac

Merge branch 'branch-24.06' into cudf-examples-cmake-fix

f984c01

minor improvements

c088772

robertmaynard requested changes Apr 17, 2024

View reviewed changes

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

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

minor improvements and add default LIB_BUILD_DIR

996367d

bdice approved these changes Apr 17, 2024

View reviewed changes

mhaseeb123 requested a review from robertmaynard April 17, 2024 19:54

robertmaynard approved these changes Apr 17, 2024

View reviewed changes

AyodeAwe approved these changes Apr 18, 2024

View reviewed changes

rapids-bot bot merged commit b8d003e into rapidsai:branch-24.06 Apr 18, 2024
75 checks passed

mhaseeb123 deleted the cudf-examples-cmake-fix branch April 18, 2024 19:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix CMake files in libcudf C++ examples to use existing libcudf build if present #15348

Fix CMake files in libcudf C++ examples to use existing libcudf build if present #15348

mhaseeb123 commented Mar 20, 2024 •

edited

Loading

copy-pr-bot bot commented Mar 20, 2024

bdice left a comment

davidwendt commented Mar 20, 2024

bdice commented Mar 20, 2024 •

edited

Loading

bdice commented Mar 20, 2024

harrism commented Mar 22, 2024

mhaseeb123 commented Mar 22, 2024

mhaseeb123 commented Apr 16, 2024

bdice left a comment

mhaseeb123 commented Apr 17, 2024

mhaseeb123 commented Apr 17, 2024 •

edited

Loading

mhaseeb123 commented Apr 17, 2024

bdice left a comment

mhaseeb123 commented Apr 18, 2024

mhaseeb123 commented Apr 18, 2024

	rapids-mamba-retry install \
	--channel "${CPP_CHANNEL}" \
	libcudf libcudf_kafka libcudf-tests

	rapids-logger "Run libcudf gtests"
	./ci/run_cudf_ctests.sh -j20
	SUITEERROR=$?

Fix CMake files in libcudf C++ examples to use existing libcudf build if present #15348

Fix CMake files in libcudf C++ examples to use existing libcudf build if present #15348

Conversation

mhaseeb123 commented Mar 20, 2024 • edited Loading

Description

Checklist

copy-pr-bot bot commented Mar 20, 2024

bdice left a comment

Choose a reason for hiding this comment

davidwendt commented Mar 20, 2024

bdice commented Mar 20, 2024 • edited Loading

bdice commented Mar 20, 2024

harrism commented Mar 22, 2024

mhaseeb123 commented Mar 22, 2024

mhaseeb123 commented Apr 16, 2024

bdice left a comment

Choose a reason for hiding this comment

mhaseeb123 commented Apr 17, 2024

mhaseeb123 commented Apr 17, 2024 • edited Loading

mhaseeb123 commented Apr 17, 2024

bdice left a comment

Choose a reason for hiding this comment

mhaseeb123 commented Apr 18, 2024

mhaseeb123 commented Apr 18, 2024

mhaseeb123 commented Mar 20, 2024 •

edited

Loading

bdice commented Mar 20, 2024 •

edited

Loading

mhaseeb123 commented Apr 17, 2024 •

edited

Loading