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

Use CTAD with Thrust function objects #9768

Merged
merged 9 commits into from
Dec 1, 2021

Conversation

codereport
Copy link
Contributor

@codereport codereport commented Nov 23, 2021

While reviewing another PR, I noticed unnecessary usage of explicit template parameters with Thrust function objects and decided to open a small PR to clean this up (CTAD showed up in C++17).

CI depends on #9766

@codereport codereport added 2 - In Progress Currently a work in progress code quality libcudf Affects libcudf (C++/CUDA) code. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Nov 23, 2021
@codereport codereport self-assigned this Nov 23, 2021
@codecov
Copy link

codecov bot commented Nov 24, 2021

Codecov Report

Merging #9768 (bf1b27e) into branch-22.02 (967a333) will decrease coverage by 0.01%.
The diff coverage is 9.24%.

❗ Current head bf1b27e differs from pull request most recent head 62c240c. Consider uploading reports for the commit 62c240c to get more accurate results
Impacted file tree graph

@@               Coverage Diff                @@
##           branch-22.02    #9768      +/-   ##
================================================
- Coverage         10.49%   10.47%   -0.02%     
================================================
  Files               119      119              
  Lines             20305    20368      +63     
================================================
+ Hits               2130     2133       +3     
- Misses            18175    18235      +60     
Impacted Files Coverage Δ
python/cudf/cudf/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/column.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/string.py 0.00% <ø> (ø)
python/cudf/cudf/core/frame.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/groupby/groupby.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/index.py 0.00% <ø> (ø)
python/cudf/cudf/core/indexed_frame.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/multiindex.py 0.00% <0.00%> (ø)
python/cudf/cudf/utils/utils.py 0.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/io/csv.py 96.00% <100.00%> (+0.10%) ⬆️
... and 1 more

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 11c3dfe...62c240c. Read the comment docs.

@codereport codereport added 3 - Ready for Review Ready for review by team 4 - Needs Review Waiting for reviewer to review or respond and removed 2 - In Progress Currently a work in progress labels Nov 24, 2021
@codereport codereport marked this pull request as ready for review November 24, 2021 04:44
@codereport codereport requested a review from a team as a code owner November 24, 2021 04:44
@@ -315,7 +315,7 @@ std::unique_ptr<cudf::column> gather(
d_out_offsets + output_count,
[] __device__(auto size) { return static_cast<size_t>(size); },
size_t{0},
thrust::plus<size_t>{});
thrust::plus{});
Copy link
Contributor

Choose a reason for hiding this comment

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

This is surprising to me.
How does it deduce the type without any parameters?

Copy link
Member

Choose a reason for hiding this comment

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

Me too. Does it somehow use the other arguments to xform_reduce?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The return type is dictated by the type of the initial value, aka size_t{0} on the line above.

@codereport codereport removed the 3 - Ready for Review Ready for review by team label Nov 24, 2021
@codereport codereport added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 4 - Needs Review Waiting for reviewer to review or respond labels Nov 24, 2021
@codereport
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 836f800 into rapidsai:branch-22.02 Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants