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.

Sign up for GitHub

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

Jump to bottom

Address some MNMG issues in cython.cu #2224

Merged
merged 2 commits into from
Apr 14, 2022
Merged

Address some MNMG issues in cython.cu #2224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67ceaf7
weights can be empty on some GPUs and not others in MNMG. Missing el…
ChuckHastings Apr 13, 2022
f406bec
add else for two other blocks of code
ChuckHastings Apr 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cpp/include/cugraph/utilities/cython.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,9 +594,10 @@ std::unique_ptr<major_minor_weights_t<vertex_t, edge_t, weight_t>> call_shuffle(
edgelist_major_vertices, // [IN / OUT]: groupby_gpu_id_and_shuffle_values() sorts in-place
vertex_t* edgelist_minor_vertices, // [IN / OUT]
weight_t* edgelist_weights, // [IN / OUT]
edge_t num_edgelist_edges);
edge_t num_edgelist_edges,
bool is_weighted);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just FYI, we're using std::optional elsewhere to address this instead of adding an additional flag. Not sure it is worth the effort in applying the same strategy here as this code will become obsolete once we switch to the C-API based approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

And yeah... this is cython, so we may not be able to use std optional here.


// Wrapper for calling renumber_edeglist() inplace:
// Wrapper for calling renumber_edgelist() inplace:
//
template <typename vertex_t, typename edge_t>
std::unique_ptr<renum_tuple_t<vertex_t, edge_t>> call_renumber(
Expand Down
25 changes: 16 additions & 9 deletions cpp/src/utilities/cython.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,8 @@ std::unique_ptr<major_minor_weights_t<vertex_t, edge_t, weight_t>> call_shuffle(
edgelist_major_vertices, // [IN / OUT]: groupby_gpu_id_and_shuffle_values() sorts in-place
vertex_t* edgelist_minor_vertices, // [IN / OUT]
weight_t* edgelist_weights, // [IN / OUT]
edge_t num_edgelist_edges)
edge_t num_edgelist_edges,
bool is_weighted)
{
auto& comm = handle.get_comms();
auto const comm_size = comm.get_size();
Expand All @@ -1197,7 +1198,7 @@ std::unique_ptr<major_minor_weights_t<vertex_t, edge_t, weight_t>> call_shuffle(
std::unique_ptr<major_minor_weights_t<vertex_t, edge_t, weight_t>> ptr_ret =
std::make_unique<major_minor_weights_t<vertex_t, edge_t, weight_t>>(handle);

if (edgelist_weights != nullptr) {
if (is_weighted) {
auto zip_edge = thrust::make_zip_iterator(
thrust::make_tuple(edgelist_major_vertices, edgelist_minor_vertices, edgelist_weights));

Expand Down Expand Up @@ -1242,7 +1243,7 @@ std::unique_ptr<major_minor_weights_t<vertex_t, edge_t, weight_t>> call_shuffle(
auto pair_first = thrust::make_zip_iterator(
thrust::make_tuple(ptr_ret->get_major().data(), ptr_ret->get_minor().data()));

auto edge_counts = (edgelist_weights != nullptr)
auto edge_counts = (is_weighted)
? cugraph::groupby_and_count(pair_first,
pair_first + ptr_ret->get_major().size(),
ptr_ret->get_weights().data(),
Expand Down Expand Up @@ -1639,42 +1640,48 @@ template std::unique_ptr<major_minor_weights_t<int32_t, int32_t, float>> call_sh
int32_t* edgelist_major_vertices,
int32_t* edgelist_minor_vertices,
float* edgelist_weights,
int32_t num_edgelist_edges);
int32_t num_edgelist_edges,
bool is_weighted);

template std::unique_ptr<major_minor_weights_t<int32_t, int64_t, float>> call_shuffle(
raft::handle_t const& handle,
int32_t* edgelist_major_vertices,
int32_t* edgelist_minor_vertices,
float* edgelist_weights,
int64_t num_edgelist_edges);
int64_t num_edgelist_edges,
bool is_weighted);

template std::unique_ptr<major_minor_weights_t<int32_t, int32_t, double>> call_shuffle(
raft::handle_t const& handle,
int32_t* edgelist_major_vertices,
int32_t* edgelist_minor_vertices,
double* edgelist_weights,
int32_t num_edgelist_edges);
int32_t num_edgelist_edges,
bool is_weighted);

template std::unique_ptr<major_minor_weights_t<int32_t, int64_t, double>> call_shuffle(
raft::handle_t const& handle,
int32_t* edgelist_major_vertices,
int32_t* edgelist_minor_vertices,
double* edgelist_weights,
int64_t num_edgelist_edges);
int64_t num_edgelist_edges,
bool is_weighted);

template std::unique_ptr<major_minor_weights_t<int64_t, int64_t, float>> call_shuffle(
raft::handle_t const& handle,
int64_t* edgelist_major_vertices,
int64_t* edgelist_minor_vertices,
float* edgelist_weights,
int64_t num_edgelist_edges);
int64_t num_edgelist_edges,
bool is_weighted);

template std::unique_ptr<major_minor_weights_t<int64_t, int64_t, double>> call_shuffle(
raft::handle_t const& handle,
int64_t* edgelist_major_vertices,
int64_t* edgelist_minor_vertices,
double* edgelist_weights,
int64_t num_edgelist_edges);
int64_t num_edgelist_edges,
bool is_weighted);

// TODO: add the remaining relevant EIDIr's:
//
Expand Down
3 changes: 2 additions & 1 deletion python/cugraph/cugraph/structure/graph_utilities.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ cdef extern from "cugraph/utilities/cython.hpp" namespace "cugraph::cython":
vertex_t *edgelist_major_vertices,
vertex_t *edgelist_minor_vertices,
weight_t* edgelist_weights,
edge_t num_edges) except +
edge_t num_edges,
bool is_weighted) except +

# 5. `renumber_edgelist()` wrapper
#
Expand Down
27 changes: 18 additions & 9 deletions python/cugraph/cugraph/structure/renumber_wrapper.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,21 @@ cdef renumber_helper(shuffled_vertices_t* ptr_maj_min_w, vertex_t, weights):
# which is not supported by '__cuda_array_interface__'
if shuffled_major_series is None:
shuffled_major_series = cudf.Series(dtype=vertex_t)
else:
shuffled_df['major_vertices']= shuffled_major_series
if shuffled_minor_series is None:
shuffled_minor_series = cudf.Series(dtype=vertex_t)
shuffled_df['major_vertices']= shuffled_major_series
shuffled_df['minor_vertices']= shuffled_minor_series
else:
shuffled_df['minor_vertices']= shuffled_minor_series

if weights is not None:
weight_t = weights.dtype
shuffled_weights_series = move_device_buffer_to_series(
move(pair_s_weights.first), weight_t, "shuffled_weights")
if shuffled_weights_series is None:
shuffled_df['value']= cudf.Series(dtype=weight_t)
shuffled_df['value']= shuffled_weights_series
else:
shuffled_df['value']= shuffled_weights_series
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we should do the same for shuffled_minor_series and shuffled_major_series

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. Wasn't causing an error, but the code is, in fact, incorrect. Updated in the last push.


return shuffled_df

Expand Down Expand Up @@ -176,7 +179,8 @@ def renumber(input_df, # maybe use cpdef ?
<int*>c_major_vertices,
<int*>c_minor_vertices,
<float*>c_edge_weights,
num_local_edges).release())
num_local_edges,
weights is not None).release())
shuffled_df = renumber_helper(ptr_shuffled_32_32_32.get(), vertex_t, weights)
major_vertices = shuffled_df['major_vertices']
minor_vertices = shuffled_df['minor_vertices']
Expand Down Expand Up @@ -245,7 +249,8 @@ def renumber(input_df, # maybe use cpdef ?
<int*>c_major_vertices,
<int*>c_minor_vertices,
<double*>c_edge_weights,
num_local_edges).release())
num_local_edges,
weights is not None).release())

shuffled_df = renumber_helper(ptr_shuffled_32_32_64.get(), vertex_t, weights)
major_vertices = shuffled_df['major_vertices']
Expand Down Expand Up @@ -317,7 +322,8 @@ def renumber(input_df, # maybe use cpdef ?
<int*>c_major_vertices,
<int*>c_minor_vertices,
<float*>c_edge_weights,
num_local_edges).release())
num_local_edges,
weights is not None).release())

shuffled_df = renumber_helper(ptr_shuffled_32_64_32.get(), vertex_t, weights)
major_vertices = shuffled_df['major_vertices']
Expand Down Expand Up @@ -387,7 +393,8 @@ def renumber(input_df, # maybe use cpdef ?
<int*>c_major_vertices,
<int*>c_minor_vertices,
<double*>c_edge_weights,
num_local_edges).release())
num_local_edges,
weights is not None).release())

shuffled_df = renumber_helper(ptr_shuffled_32_64_64.get(), vertex_t, weights)
major_vertices = shuffled_df['major_vertices']
Expand Down Expand Up @@ -459,7 +466,8 @@ def renumber(input_df, # maybe use cpdef ?
<long*>c_major_vertices,
<long*>c_minor_vertices,
<float*>c_edge_weights,
num_local_edges).release())
num_local_edges,
weights is not None).release())

shuffled_df = renumber_helper(ptr_shuffled_64_64_32.get(), vertex_t, weights)
major_vertices = shuffled_df['major_vertices']
Expand Down Expand Up @@ -530,7 +538,8 @@ def renumber(input_df, # maybe use cpdef ?
<long*>c_major_vertices,
<long*>c_minor_vertices,
<double*>c_edge_weights,
num_local_edges).release())
num_local_edges,
weights is not None).release())

shuffled_df = renumber_helper(ptr_shuffled_64_64_64.get(), vertex_t, weights)
major_vertices = shuffled_df['major_vertices']
Expand Down