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 benchmarks coded in namespace cudf and using namespace cudf #12915

Merged

Conversation

karthikeyann
Copy link
Contributor

Description

Fixes BINARYOP_BENCH, GROUPBY_NVBENCH benchmarks from using namespace cudf directly.
Fixes SCATTER_LISTS_BENCH benchmarks coded in namespace cudf

Reference #11734

Checklist

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

@karthikeyann karthikeyann added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 9, 2023
@karthikeyann karthikeyann requested a review from a team as a code owner March 9, 2023 08:18
@karthikeyann karthikeyann self-assigned this Mar 9, 2023
Copy link
Member

@divyegala divyegala left a comment

Choose a reason for hiding this comment

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

LGTM, with a minor comment

Comment on lines +44 to +46
const cudf::size_type base_size{(cudf::size_type)state.range(0)};
const cudf::size_type num_elements_per_row{(cudf::size_type)state.range(1)};
const auto num_rows = (cudf::size_type)ceil(double(base_size) / num_elements_per_row);
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if it's in the guide, but I have seen libcudf devs using east-const by default. However, if it's a matter of personal preference then it's fine with me

Copy link
Contributor

Choose a reason for hiding this comment

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

Recommend to change that too.

@karthikeyann
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 9ec5477 into rapidsai:branch-23.04 Mar 9, 2023
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 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