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

Test select_random_vertices for all possible values of flags #4042

Merged
merged 14 commits into from
Dec 12, 2023
Merged

Test select_random_vertices for all possible values of flags #4042

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c568fd0
Update select_random_vertices to select as many random vetices local …
Dec 5, 2023
b2dd307
Update select_random_vertices to select as many random vetices local …
Dec 5, 2023
77b0666
Update select_random_vertices to select as many random vetices local …
Dec 5, 2023
aad9372
Update select_random_vertices to select as many random vetices local …
Dec 5, 2023
028610d
Update comments and fix logic
Dec 5, 2023
10d8599
Update test
Dec 5, 2023
70da144
Update test
Dec 5, 2023
4799803
Update test
Dec 5, 2023
1478f4a
Merge branch 'branch-24.02' of github.com:rapidsai/cugraph into updat…
Dec 12, 2023
3fb3973
Undo changes
Dec 12, 2023
7692477
Undo changes
Dec 12, 2023
0573351
Undo changes
Dec 12, 2023
e866560
Check all possible flag values
Dec 12, 2023
e827bd7
Set check_correctness to false for benchmark test
Dec 12, 2023
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
6 changes: 5 additions & 1 deletion cpp/include/cugraph/graph_functions.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,9 @@ weight_t compute_total_edge_weight(
* @param select_count The number of vertices to select from the graph
* @param with_replacement If true, select with replacement, if false select without replacement
* @param sort_vertices If true, return the sorted vertices (in the ascending order).
* @param shuffle_int_to_local If true and If @p given_set is not specified
* then shuffle internal (i.e. renumbered) vertices to their local GPUs based on vertex
* partitioning, otherwise shuffle as many vertices as local vertex partition size to each GPU.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why should we care about renumbering here? And why apply this flag only when @p given_set is not specified?

Copy link
Contributor Author

@naimnv naimnv Dec 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think I have to change all of these.
Objective: For a graph with N vertices, we want to generate N unique random numbers in range [0, N) (across GPUs) and we don't want to shuffle the generated numbers to GPUs based on vertex partitioning, and each GPU gets graph_view.local_vertex_partition_range_size() many random numbers,
while keeping supporting the existing behavior of select_random_vertices.

Copy link
Contributor Author

@naimnv naimnv Dec 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Does it make sense now @seunghwak ?

Copy link
Collaborator

@ChuckHastings ChuckHastings Dec 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I guess I'm wondering why this even needs to be part of the select_random_vertices call.

While there is randomness to the feature that we need, I think all that we need is a "distributed shuffle" function. Given an rmm::device_uvector<T> on each GPU that is initialized with some values, we want the equivalent of thrust::shuffle of that distributed device vector.

I'm not aware of an algorithm that would need to both select a random subset of the vertex list and randomly order that list across all of the GPUs. If there was such an algorithm it could first call select_random_vertices and then call a new randomly shuffle a distributed device vector function.

I don't see a benefit of conflating the two actions into a single function.

Copy link
Contributor Author

@naimnv naimnv Dec 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Changes havebeen be aborted. Shuffling will be done by the ECG implementation itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

If this PR is not meant to be merged, should we better close this PR?

Copy link
Contributor Author

@naimnv naimnv Dec 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Actually all the changes to the select_random_vertices has been aborted, but the test has been updated. Please review the tests.

* @return Device vector of selected vertices.
*/
template <typename vertex_t, typename edge_t, bool store_transposed, bool multi_gpu>
Expand All @@ -914,7 +917,8 @@ rmm::device_uvector<vertex_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool do_expensive_check = false);
bool shuffle_int_to_local = true,
bool do_expensive_check = false);

/**
* @brief renumber sampling output
Expand Down
45 changes: 43 additions & 2 deletions cpp/src/structure/select_random_vertices_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ rmm::device_uvector<vertex_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check)
{
size_t num_of_elements_in_given_set{0};
Expand Down Expand Up @@ -232,8 +233,48 @@ rmm::device_uvector<vertex_t> select_random_vertices(
}

if constexpr (multi_gpu) {
mg_sample_buffer = cugraph::detail::shuffle_int_vertices_to_local_gpu_by_vertex_partitioning(
handle, std::move(mg_sample_buffer), partition_range_lasts);
if (given_set) {
mg_sample_buffer = cugraph::detail::shuffle_int_vertices_to_local_gpu_by_vertex_partitioning(
handle, std::move(mg_sample_buffer), partition_range_lasts);

} else {
if (!shuffle_int_to_local &&
select_count == static_cast<size_t>(graph_view.number_of_vertices())) {
// shuffle as many vertices as local vertex partition size to each GPU.
auto& comm = handle.get_comms();
auto const comm_size = comm.get_size();
auto const comm_rank = comm.get_rank();
std::vector<size_t> tx_value_counts(comm_size, 0);
auto sample_buffer_sizes = cugraph::host_scalar_allgather(
handle.get_comms(), mg_sample_buffer.size(), handle.get_stream());

auto expected_sample_buffer_sizes = cugraph::host_scalar_allgather(
handle.get_comms(), graph_view.local_vertex_partition_range_size(), handle.get_stream());

std::vector<size_t> nr_smaples(comm_size, 0);

// find out how many elements current GPU needs to send to other GPUs
for (int i = 0; i < comm_size; i++) {
size_t nr_samples_ith_gpu = sample_buffer_sizes[i];
for (int j = 0; nr_samples_ith_gpu > 0 && j < comm_size; j++) {
if (expected_sample_buffer_sizes[j] > static_cast<vertex_t>(nr_smaples[j])) {
size_t delta =
std::min(nr_samples_ith_gpu, expected_sample_buffer_sizes[j] - nr_smaples[j]);
if (comm_rank == i) { tx_value_counts[j] = delta; }
nr_smaples[j] += delta;
nr_samples_ith_gpu -= delta;
}
}
}

std::tie(mg_sample_buffer, std::ignore) = cugraph::shuffle_values(
handle.get_comms(), mg_sample_buffer.begin(), tx_value_counts, handle.get_stream());
} else {
mg_sample_buffer =
cugraph::detail::shuffle_int_vertices_to_local_gpu_by_vertex_partitioning(
handle, std::move(mg_sample_buffer), partition_range_lasts);
}
}
}

if (given_set) {
Expand Down
6 changes: 6 additions & 0 deletions cpp/src/structure/select_random_vertices_mg.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ template rmm::device_uvector<int32_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check);

template rmm::device_uvector<int32_t> select_random_vertices(
Expand All @@ -36,6 +37,7 @@ template rmm::device_uvector<int32_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check);

template rmm::device_uvector<int64_t> select_random_vertices(
Expand All @@ -46,6 +48,7 @@ template rmm::device_uvector<int64_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check);

template rmm::device_uvector<int32_t> select_random_vertices(
Expand All @@ -56,6 +59,7 @@ template rmm::device_uvector<int32_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check);

template rmm::device_uvector<int32_t> select_random_vertices(
Expand All @@ -66,6 +70,7 @@ template rmm::device_uvector<int32_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check);

template rmm::device_uvector<int64_t> select_random_vertices(
Expand All @@ -76,6 +81,7 @@ template rmm::device_uvector<int64_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check);

} // namespace cugraph
6 changes: 6 additions & 0 deletions cpp/src/structure/select_random_vertices_sg.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ template rmm::device_uvector<int32_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check);

template rmm::device_uvector<int32_t> select_random_vertices(
Expand All @@ -36,6 +37,7 @@ template rmm::device_uvector<int32_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check);

template rmm::device_uvector<int64_t> select_random_vertices(
Expand All @@ -46,6 +48,7 @@ template rmm::device_uvector<int64_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check);

template rmm::device_uvector<int32_t> select_random_vertices(
Expand All @@ -56,6 +59,7 @@ template rmm::device_uvector<int32_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check);

template rmm::device_uvector<int32_t> select_random_vertices(
Expand All @@ -66,6 +70,7 @@ template rmm::device_uvector<int32_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check);

template rmm::device_uvector<int64_t> select_random_vertices(
Expand All @@ -76,6 +81,7 @@ template rmm::device_uvector<int64_t> select_random_vertices(
size_t select_count,
bool with_replacement,
bool sort_vertices,
bool shuffle_int_to_local,
bool do_expensive_check);

} // namespace cugraph
2 changes: 1 addition & 1 deletion cpp/tests/structure/mg_select_random_vertices_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ class Tests_MGSelectRandomVertices
//

for (int idx = 0; idx < with_replacement_flags.size(); idx++) {
bool with_replacement = false;
bool with_replacement = with_replacement_flags[idx];
auto d_sampled_vertices = cugraph::select_random_vertices(
*handle_,
mg_graph_view,
Expand Down