Switch over to rapids-cmake patches for thrust #11921

robertmaynard · 2022-10-14T14:11:40Z

Description

Now that rapids-cmake supports custom patches we can move cudf over to rapids-cmake for Thrust. This removes the need for custom install rules in cudf for Thrust, as rapids-cmake does that for us.

This also separates out all Thrust patches so that we can better track upstream approval and remove as needed.

Checklist

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

Allows us to have better history tracking of thrust changes, and keep cudf working with rapids-cmake thrust changes

robertmaynard · 2022-10-14T14:12:25Z

~~Currently blocked by rapidsai/rapids-cmake#291 and rapidsai/rapids-cmake#292~~

robertmaynard · 2022-10-14T14:15:46Z

Manually verified that this change hasn't regressed libcudf binary size ( 262MB for single arch ) and compile times are within a couple of seconds of branch-22.12.

branch-22.12  wall clock 12m 37s
this pr       wall clock 12m 49s

codecov · 2022-10-14T16:35:33Z

Codecov Report

Base: 87.40% // Head: 86.89% // Decreases project coverage by -0.51% ⚠️

Coverage data is based on head (5f534e4) compared to base (f72c4ce).
Patch coverage: 86.61% of modified lines in pull request are covered.

Additional details and impacted files

@@               Coverage Diff                @@
##           branch-22.12   #11921      +/-   ##
================================================
- Coverage         87.40%   86.89%   -0.52%     
================================================
  Files               133      133              
  Lines             21833    21982     +149     
================================================
+ Hits              19084    19101      +17     
- Misses             2749     2881     +132

Impacted Files	Coverage Δ
python/cudf/cudf/core/udf/__init__.py	`50.00% <ø> (ø)`
python/cudf/cudf/io/orc.py	`92.94% <ø> (-0.09%)`	⬇️
...thon/dask_cudf/dask_cudf/tests/test_distributed.py	`18.86% <ø> (+4.94%)`	⬆️
python/cudf/cudf/core/_base_index.py	`82.20% <43.75%> (-3.35%)`	⬇️
python/cudf/cudf/io/text.py	`91.66% <66.66%> (-8.34%)`	⬇️
python/strings_udf/strings_udf/__init__.py	`84.31% <76.00%> (-12.57%)`	⬇️
python/cudf/cudf/core/index.py	`92.96% <95.16%> (+0.33%)`	⬆️
python/cudf/cudf/__init__.py	`90.69% <100.00%> (ø)`
python/cudf/cudf/core/column/categorical.py	`89.34% <100.00%> (ø)`
python/cudf/cudf/core/groupby/groupby.py	`91.30% <100.00%> (+0.08%)`	⬆️
... and 28 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

cpp/cmake/thirdparty/patches/thrust_override.json

bdice

I love this change. It will be so much easier to test and update our patches in the future.

cpp/cmake/thirdparty/patches/thrust_override.json

Co-authored-by: Mark Harris <mharris@nvidia.com>

Co-authored-by: Bradley Dice <bdice@bradleydice.com>

bdice

Awesome. Thanks.

robertmaynard · 2022-10-24T14:26:07Z

@gpucibot merge

Switch over to rapids-cmake patches for thrust

ed067d6

Allows us to have better history tracking of thrust changes, and keep cudf working with rapids-cmake thrust changes

robertmaynard added feature request New feature or request 0 - Blocked Cannot progress due to external reasons non-breaking Non-breaking change labels Oct 14, 2022

robertmaynard requested a review from a team as a code owner October 14, 2022 14:11

github-actions bot added CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. labels Oct 14, 2022

Correct style issues found by CI

903eb17

robertmaynard added 3 - Ready for Review Ready for review by team and removed 0 - Blocked Cannot progress due to external reasons labels Oct 14, 2022