Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for logical and syntactical errors in libcudf c++ examples #15346

Merged
merged 13 commits into from
Apr 10, 2024

Conversation

mhaseeb123
Copy link
Member

@mhaseeb123 mhaseeb123 commented Mar 20, 2024

Description

This PR fixes a couple of fatal compile and runtime errors in libcudf/strings examples

Checklist

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

@mhaseeb123 mhaseeb123 requested a review from a team as a code owner March 20, 2024 03:22
Copy link

copy-pr-bot bot commented Mar 20, 2024

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.

@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Mar 20, 2024
@mhaseeb123 mhaseeb123 added 3 - Ready for Review Ready for review by team non-breaking Non-breaking change Reliability bug Something isn't working and removed Reliability labels Mar 20, 2024
@ttnghia
Copy link
Contributor

ttnghia commented Mar 20, 2024

/ok to test

Copy link
Contributor

@davidwendt davidwendt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this.

cpp/examples/strings/common.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to add something to our CI to ensure these examples are run? They should be building already, as part of libcudf-example.

- name: libcudf-example
version: {{ version }}
script: install_libcudf_example.sh

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI logs of a recent build show that this was failing but not triggering a failure for the conda build.
https://github.com/rapidsai/cudf/actions/runs/8351275479/job/22859341825#step:7:4337

-- Build files have been written to: /opt/conda/conda-bld/work/cpp/examples/strings/build
[1/8] Building CUDA object CMakeFiles/custom_optimized.dir/custom_optimized.cu.o
FAILED: CMakeFiles/custom_optimized.dir/custom_optimized.cu.o 
/opt/conda/conda-bld/_build_env/bin/nvcc -forward-unknown-to-host-compiler -DFMT_HEADER_ONLY=1 -DLIBCUDACXX_ENABLE_EXPERIMENTAL_MEMORY_RESOURCE -DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_INFO -DSPDLOG_FMT_EXTERNAL -DTHRUST_DEVICE_SYSTEM=THRUST_DEVICE_SYSTEM_CUDA -DTHRUST_DISABLE_ABI_NAMESPACE -DTHRUST_HOST_SYSTEM=THRUST_HOST_SYSTEM_CPP -DTHRUST_IGNORE_ABI_NAMESPACE_ERROR -I/opt/conda/conda-bld/work/cpp/build/_deps/cccl-src/thrust/thrust/cmake/../.. -I/opt/conda/conda-bld/work/cpp/build/_deps/cccl-src/libcudacxx/lib/cmake/libcudacxx/../../../include -I/opt/conda/conda-bld/work/cpp/build/_deps/cccl-src/cub/cub/cmake/../.. -isystem /opt/conda/conda-bld/work/cpp/build/_deps/dlpack-src/include -isystem /opt/conda/conda-bld/work/cpp/build/_deps/jitify-src -isystem /opt/conda/conda-bld/work/cpp/include -isystem /opt/conda/conda-bld/work/cpp/build/include -isystem /opt/conda/conda-bld/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/include -isystem /opt/conda/conda-bld/_build_env/targets/x86_64-linux/include "--generate-code=arch=compute_52,code=[compute_52,sm_52]" -Xcompiler=-fPIE --expt-extended-lambda --expt-relaxed-constexpr -MD -MT CMakeFiles/custom_optimized.dir/custom_optimized.cu.o -MF CMakeFiles/custom_optimized.dir/custom_optimized.cu.o.d -x cu -c /opt/conda/conda-bld/work/cpp/examples/strings/custom_optimized.cu -o CMakeFiles/custom_optimized.dir/custom_optimized.cu.o
/opt/conda/conda-bld/work/cpp/examples/strings/custom_optimized.cu(157): error: no instance of overloaded function "cudf::make_strings_column" matches the argument list
            argument types are: (cudf::size_type, rmm::device_uvector<cudf::size_type>, rmm::device_buffer, {...}, int)
    auto result = cudf::make_strings_column(names.size(), std::move(offsets), chars.release(), {}, 0);
                  ^

1 error detected in the compilation of "/opt/conda/conda-bld/work/cpp/examples/strings/custom_optimized.cu".

Yes. We need to add set -euo pipefail (aligning with our typical set of CI shell script flags) to cpp/examples/build.sh to ensure that compilation failures aren't allowed. Can we do that in this PR? Please push an error that would test it to make sure that compilation failures trigger CI failure.

@mhaseeb123
Copy link
Member Author

The CI logs of a recent build show that this was failing but not triggering a failure for the conda build. https://github.com/rapidsai/cudf/actions/runs/8351275479/job/22859341825#step:7:4337

-- Build files have been written to: /opt/conda/conda-bld/work/cpp/examples/strings/build
[1/8] Building CUDA object CMakeFiles/custom_optimized.dir/custom_optimized.cu.o
FAILED: CMakeFiles/custom_optimized.dir/custom_optimized.cu.o 
/opt/conda/conda-bld/_build_env/bin/nvcc -forward-unknown-to-host-compiler -DFMT_HEADER_ONLY=1 -DLIBCUDACXX_ENABLE_EXPERIMENTAL_MEMORY_RESOURCE -DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_INFO -DSPDLOG_FMT_EXTERNAL -DTHRUST_DEVICE_SYSTEM=THRUST_DEVICE_SYSTEM_CUDA -DTHRUST_DISABLE_ABI_NAMESPACE -DTHRUST_HOST_SYSTEM=THRUST_HOST_SYSTEM_CPP -DTHRUST_IGNORE_ABI_NAMESPACE_ERROR -I/opt/conda/conda-bld/work/cpp/build/_deps/cccl-src/thrust/thrust/cmake/../.. -I/opt/conda/conda-bld/work/cpp/build/_deps/cccl-src/libcudacxx/lib/cmake/libcudacxx/../../../include -I/opt/conda/conda-bld/work/cpp/build/_deps/cccl-src/cub/cub/cmake/../.. -isystem /opt/conda/conda-bld/work/cpp/build/_deps/dlpack-src/include -isystem /opt/conda/conda-bld/work/cpp/build/_deps/jitify-src -isystem /opt/conda/conda-bld/work/cpp/include -isystem /opt/conda/conda-bld/work/cpp/build/include -isystem /opt/conda/conda-bld/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/include -isystem /opt/conda/conda-bld/_build_env/targets/x86_64-linux/include "--generate-code=arch=compute_52,code=[compute_52,sm_52]" -Xcompiler=-fPIE --expt-extended-lambda --expt-relaxed-constexpr -MD -MT CMakeFiles/custom_optimized.dir/custom_optimized.cu.o -MF CMakeFiles/custom_optimized.dir/custom_optimized.cu.o.d -x cu -c /opt/conda/conda-bld/work/cpp/examples/strings/custom_optimized.cu -o CMakeFiles/custom_optimized.dir/custom_optimized.cu.o
/opt/conda/conda-bld/work/cpp/examples/strings/custom_optimized.cu(157): error: no instance of overloaded function "cudf::make_strings_column" matches the argument list
            argument types are: (cudf::size_type, rmm::device_uvector<cudf::size_type>, rmm::device_buffer, {...}, int)
    auto result = cudf::make_strings_column(names.size(), std::move(offsets), chars.release(), {}, 0);
                  ^

1 error detected in the compilation of "/opt/conda/conda-bld/work/cpp/examples/strings/custom_optimized.cu".

Yes. We need to add set -euo pipefail (aligning with our typical set of CI shell script flags) to cpp/examples/build.sh to ensure that compilation failures aren't allowed. Can we do that in this PR? Please push an error that would test it to make sure that compilation failures trigger CI failure.

Sounds good, @bdice. Pushing in an error to test the CI in a commit now.

@bdice
Copy link
Contributor

bdice commented Mar 20, 2024

/ok to test

1 similar comment
@mhaseeb123
Copy link
Member Author

/ok to test

@mhaseeb123 mhaseeb123 changed the title fix for libcudf examples Fix for logical and syntactical errors in libcudf c++ examples Mar 22, 2024
Including strings_column_view header to fix build error
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one small suggestion.

cpp/examples/basic/src/process_csv.cpp Outdated Show resolved Hide resolved
@mhaseeb123
Copy link
Member Author

/ok to test

@mhaseeb123
Copy link
Member Author

/ok to test

@mhaseeb123
Copy link
Member Author

@bdice looks like the CI is now failing at the #error https://github.com/rapidsai/cudf/actions/runs/8607963757/job/23589503980?pr=15346#step:7:4992. Going to remove that line and re-trigger CI

@mhaseeb123
Copy link
Member Author

/ok to test

@mhaseeb123
Copy link
Member Author

/ok to test

@mhaseeb123
Copy link
Member Author

/merge

@rapids-bot rapids-bot bot merged commit 15c148d into rapidsai:branch-24.06 Apr 10, 2024
70 checks passed
@mhaseeb123 mhaseeb123 deleted the cudf-examples-udpate branch April 10, 2024 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants