-
Notifications
You must be signed in to change notification settings - Fork 341
Compile Optimizations - batch 3 #5119
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
Compile Optimizations - batch 3 #5119
Conversation
…hrust has fixed the issue
cpp/src/community/k_truss_impl.cuh
Outdated
| std::nullopt, | ||
| std::nullopt, | ||
| std::nullopt, | ||
| std::move(edge_properties), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have shuffle_ext_edges. Should we better create shuffle_int_edges as well instead of calling this detail space function in the application code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I meant to do that. I'll update this in the next push.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added shuffle_int_edges
cpp/src/detail/shuffle_wrappers.hpp
Outdated
| namespace cugraph { | ||
| namespace detail { | ||
|
|
||
| // TODO (in this PR): move this file to the src/detail directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have already moves this file, so the first line of this TODO statement is no longer valid, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
cpp/tests/prims/mg_per_v_pair_transform_dst_nbr_intersection.cu
Outdated
Show resolved
Hide resolved
cpp/tests/prims/mg_per_v_pair_transform_dst_nbr_weighted_intersection.cu
Outdated
Show resolved
Hide resolved
cpp/tests/structure/mg_has_edge_and_compute_multiplicity_test.cpp
Outdated
Show resolved
Hide resolved
…4' into variant_work_part4
seunghwak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except for the bug (start=>end).
| if (edge_id_) tmp_edge_properties.push_back(std::move((*edge_id_)[0])); | ||
| if (edge_type_) tmp_edge_properties.push_back(std::move((*edge_type_)[0])); | ||
| if (edge_start_time_) tmp_edge_properties.push_back(std::move((*edge_start_time_)[0])); | ||
| if (edge_start_time_) tmp_edge_properties.push_back(std::move((*edge_start_time_)[0])); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
start_time=>end_time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Fix erroneous assertion (closes rapidsai#5138). Authors: - Seunghwa Kang (https://github.com/seunghwak) - Chuck Hastings (https://github.com/ChuckHastings) Approvers: - Chuck Hastings (https://github.com/ChuckHastings) URL: rapidsai#5142
|
/merge |
Next round of compile optimizations. Big focus on shuffle operations.
Breaking since it deletes some deprecated functions.
Aggregate reduction in size of libcugraph.so (with first two batches): 9%