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

Rename multiple .cuh (.cu) files to .hpp (.cpp) #2501

Merged
merged 7 commits into from
Aug 11, 2022

Conversation

seunghwak
Copy link
Contributor

Pre-requisites for #2479

Several .cuh header files (.cuh indicates nvcc is required to compile) can be renamed to .hpp (.hpp indicates the file can be compiled with a host compiler such as g++) with no or minimum modifications.

This PR performs multiple renaming (eventually to separate edge_partition_src_dst_property.cuh to .hpp and .cuh files).

Breaking as public header files are renamed.

@seunghwak seunghwak added 3 - Ready for Review improvement Improvement / enhancement to an existing function breaking Breaking change labels Aug 3, 2022
@seunghwak seunghwak added this to the 22.10 milestone Aug 3, 2022
@seunghwak seunghwak requested review from a team as code owners August 3, 2022 18:43
@seunghwak seunghwak self-assigned this Aug 3, 2022
@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2022

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.10@c84c8bf). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 4d5a807 differs from pull request most recent head 74b6697. Consider uploading reports for the commit 74b6697 to get more accurate results

@@               Coverage Diff               @@
##             branch-22.10    #2501   +/-   ##
===============================================
  Coverage                ?   61.26%           
===============================================
  Files                   ?      106           
  Lines                   ?     5404           
  Branches                ?        0           
===============================================
  Hits                    ?     3311           
  Misses                  ?     2093           
  Partials                ?        0           

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

Comment on lines +155 to +169
template <typename Iterator, typename TupleType, size_t I, size_t N>
struct atomic_accumulate_thrust_tuple_impl {
__device__ constexpr void compute(Iterator iter, TupleType const& value) const
{
atomic_accumulate_impl(thrust::raw_reference_cast(thrust::get<I>(*iter)),
thrust::get<I>(value));
atomic_accumulate_thrust_tuple_impl<Iterator, TupleType, I + 1, N>().compute(iter, value);
}
};

template <typename Iterator, typename TupleType, size_t I>
struct atomic_accumulate_thrust_tuple_impl<Iterator, TupleType, I, I> {
__device__ constexpr void compute(Iterator iter, TupleType const& value) const {}
};

Copy link
Contributor

@naimnv naimnv Aug 4, 2022

Choose a reason for hiding this comment

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

@seunghwak If the tuple has too many element (not sure if it can be the case), would it be a bit inefficient in terms of number of function instantiates it would generate? (Not related to this PR though)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we discussed offline, this will end up with N atomicAdd instructions. So, no in terms of execution time, but yes, this can increase compile time.

@ChuckHastings
Copy link
Collaborator

@gpucibot merge

@rapids-bot rapids-bot bot merged commit cc05758 into rapidsai:branch-22.10 Aug 11, 2022
@seunghwak seunghwak deleted the enh_edge_src_dst_property branch August 11, 2022 23:23
@seunghwak seunghwak restored the enh_edge_src_dst_property branch August 18, 2022 20:36
@seunghwak seunghwak deleted the enh_edge_src_dst_property branch August 25, 2022 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change improvement Improvement / enhancement to an existing function
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants