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

Update edge triangle count to call a non detail primitive #4630

Conversation

jnke2016
Copy link
Contributor

Create a non detail primitive that iterates over each input vertex pair and returns the common destination neighbor list
pair in a CSR-like format.

closes #3475

@jnke2016 jnke2016 requested a review from a team as a code owner August 24, 2024 01:23
@jnke2016 jnke2016 self-assigned this Aug 24, 2024
@jnke2016 jnke2016 added this to the 24.10 milestone Aug 24, 2024
@jnke2016 jnke2016 changed the title Update edge triangle count to call non detail primitive Update edge triangle count to call a non detail primitive Aug 24, 2024

namespace cugraph {

namespace detail {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't need to define an empty detail namespace.


#include <rmm/exec_policy.hpp>

#include <thrust/binary_search.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think any of these thrust includes are necessary.

You might want to review all of your includes to reduce them just to what's directly used in this file.

@@ -0,0 +1,105 @@
/*
* Copyright (c) 2022-2024, NVIDIA CORPORATION.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably just be 2024.

{
static_assert(!GraphViewType::is_storage_transposed);

if (do_expensive_check) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason to duplicate this error check? Couldn't you just pass do_expensive_check directly to the detail::nbr_intersection and let it do this check (it already does).

"Invalid input arguments: there are invalid input vertex pairs.");
}

auto [intersection_offsets, intersection_indices] =
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should be able to just return the results directly...

return detail::nbr_intersection(handle,
...

instead of capturing and then immediately returning the result.

@ChuckHastings ChuckHastings added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 3, 2024
cpp/src/community/edge_triangle_count_impl.cuh Outdated Show resolved Hide resolved
cpp/src/prims/per_v_pair_dst_nbr_intersection.cuh Outdated Show resolved Hide resolved
* @param graph_view Non-owning graph object.
* @param vertex_pair_first Iterator pointing to the first (inclusive) input vertex pair.
* @param vertex_pair_last Iterator pointing to the last (exclusive) input vertex pair.
* @param A flag to run expensive checks for input arguments (if set to `true`).
Copy link
Contributor

Choose a reason for hiding this comment

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

Documentation for return values is missing (@return)

@ChuckHastings
Copy link
Collaborator

/merge

@rapids-bot rapids-bot bot merged commit 8009309 into rapidsai:branch-24.10 Sep 11, 2024
109 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuGraph improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a non-detail namespace wrapper for detail::nbr_intersection
3 participants