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

Move rank scan implementations from scan_inclusive.cu to rank_scan.cu #9351

Merged
merged 3 commits into from
Oct 5, 2021

Conversation

davidwendt
Copy link
Contributor

This change was mainly to improve the compile time for reductions/scan/scan_inclusive.cu by refactoring out the rank-scan functions into a separate file rank.cu. Although the overall compile time improvement for scan_inclusive.cu is only 25%, the source code is better organized with this change. The code function has changed.

The detail inclusive_rank_scan and inclusive_dense_rank_scan declarations were moved from src/reductions/scan/scan.cuh to include/cudf/detail/scan.hpp and dispatching of the RANK and DENSE_RANK aggregation is done in scan.cpp instead of handled by scan_inclusive.cu and also scan_exclusive.cu (which just throws an exception anyway).

@davidwendt davidwendt added 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 30, 2021
@davidwendt davidwendt self-assigned this Sep 30, 2021
@github-actions github-actions bot added the CMake CMake build issue label Sep 30, 2021
@codecov
Copy link

codecov bot commented Sep 30, 2021

Codecov Report

Merging #9351 (9a7240a) into branch-21.12 (ab4bfaa) will decrease coverage by 0.04%.
The diff coverage is 2.07%.

❗ Current head 9a7240a differs from pull request most recent head 7c98115. Consider uploading reports for the commit 7c98115 to get more accurate results
Impacted file tree graph

@@               Coverage Diff                @@
##           branch-21.12    #9351      +/-   ##
================================================
- Coverage         10.79%   10.74%   -0.05%     
================================================
  Files               116      116              
  Lines             18869    19038     +169     
================================================
+ Hits               2036     2045       +9     
- Misses            16833    16993     +160     
Impacted Files Coverage Δ
python/cudf/cudf/core/_base_index.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/categorical.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/column.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/datetime.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/numerical.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/string.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/timedelta.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/dataframe.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/frame.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/index.py 0.00% <0.00%> (ø)
... and 19 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 91f1dea...7c98115. Read the comment docs.

@davidwendt davidwendt added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Oct 4, 2021
@davidwendt davidwendt marked this pull request as ready for review October 4, 2021 12:35
@davidwendt davidwendt requested review from a team as code owners October 4, 2021 12:35
@harrism
Copy link
Member

harrism commented Oct 5, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 122da20 into rapidsai:branch-21.12 Oct 5, 2021
@davidwendt davidwendt deleted the split-up-incl-scan branch October 5, 2021 12:28
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 CMake CMake build issue 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.

4 participants