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

Fix eigenvector testing and HITS testing discrepancies #3979

Merged
merged 5 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion cpp/src/centrality/eigenvector_centrality_impl.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ rmm::device_uvector<weight_t> eigenvector_centrality(
centralities.end(),
old_centralities.data());

update_edge_src_property(handle, pull_graph_view, centralities.begin(), edge_src_centralities);
update_edge_src_property(
handle, pull_graph_view, old_centralities.begin(), edge_src_centralities);

if (edge_weight_view) {
per_v_transform_reduce_incoming_e(
Expand All @@ -122,6 +123,13 @@ rmm::device_uvector<weight_t> eigenvector_centrality(
centralities.begin());
}

thrust::transform(handle.get_thrust_policy(),
centralities.begin(),
centralities.end(),
old_centralities.begin(),
centralities.begin(),
thrust::plus<weight_t>());

// Normalize the centralities
auto hypotenuse = sqrt(transform_reduce_v(
handle,
Expand Down
17 changes: 10 additions & 7 deletions cpp/src/link_analysis/hits_impl.cuh
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ std::tuple<result_t, size_t> hits(raft::handle_t const& handle,
prev_hubs + graph_view.local_vertex_partition_range_size(),
result_t{1.0} / num_vertices);
}
for (size_t iter = 0; iter < max_iterations; ++iter) {
size_t iter{0};
while (true) {
// Update current destination authorities property
per_v_transform_reduce_incoming_e(
handle,
Expand Down Expand Up @@ -162,17 +163,19 @@ std::tuple<result_t, size_t> hits(raft::handle_t const& handle,
thrust::make_zip_iterator(thrust::make_tuple(curr_hubs, prev_hubs)),
[] __device__(auto, auto val) { return std::abs(thrust::get<0>(val) - thrust::get<1>(val)); },
result_t{0});
if (diff_sum < epsilon) {
final_iteration_count = iter;
std::swap(prev_hubs, curr_hubs);
break;
}

update_edge_src_property(handle, graph_view, curr_hubs, prev_src_hubs);

// Swap pointers for the next iteration
// After this swap call, prev_hubs has the latest value of hubs
std::swap(prev_hubs, curr_hubs);
iter++;

if (diff_sum < epsilon) {
break;
} else if (iter >= max_iterations) {
CUGRAPH_FAIL("HITS failed to converge.");
}
}

if (normalize) {
Expand All @@ -188,7 +191,7 @@ std::tuple<result_t, size_t> hits(raft::handle_t const& handle,
hubs);
}

return std::make_tuple(diff_sum, final_iteration_count);
return std::make_tuple(diff_sum, iter);
}

} // namespace detail
Expand Down
19 changes: 19 additions & 0 deletions cpp/tests/c_api/eigenvector_centrality_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,30 @@ int test_eigenvector_centrality()
h_src, h_dst, h_wgt, h_result, num_vertices, num_edges, TRUE, epsilon, max_iterations);
}

int test_eigenvector_centrality_3971()
{
size_t num_edges = 4;
size_t num_vertices = 3;

vertex_t h_src[] = {0, 1, 1, 2};
vertex_t h_dst[] = {1, 0, 2, 1};
weight_t h_wgt[] = {1.0f, 1.0f, 1.0f, 1.0f};
weight_t h_result[] = {0.5, 0.707107, 0.5};

double epsilon = 1e-6;
size_t max_iterations = 1000;

// Eigenvector centrality wants store_transposed = TRUE
return generic_eigenvector_centrality_test(
h_src, h_dst, h_wgt, h_result, num_vertices, num_edges, TRUE, epsilon, max_iterations);
}

/******************************************************************************/

int main(int argc, char** argv)
{
int result = 0;
result |= RUN_TEST(test_eigenvector_centrality);
result |= RUN_TEST(test_eigenvector_centrality_3971);
return result;
}
1 change: 0 additions & 1 deletion cpp/tests/centrality/eigenvector_centrality_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ void eigenvector_centrality_reference(vertex_t const* src,
size_t iter{0};
while (true) {
std::copy(tmp_centralities.begin(), tmp_centralities.end(), old_centralities.begin());
std::fill(tmp_centralities.begin(), tmp_centralities.end(), double{0});

for (size_t e = 0; e < num_edges; ++e) {
auto w = weights ? (*weights)[e] : weight_t{1.0};
Expand Down
4 changes: 2 additions & 2 deletions cpp/tests/link_analysis/hits_test.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2022, NVIDIA CORPORATION.
* Copyright (c) 2020-2023, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -176,7 +176,7 @@ class Tests_Hits : public ::testing::TestWithParam<std::tuple<Hits_Usecase, inpu

auto graph_view = graph.view();
auto maximum_iterations = 500;
weight_t tolerance = 1e-8;
weight_t tolerance = 1e-5;
rmm::device_uvector<weight_t> d_hubs(graph_view.local_vertex_partition_range_size(),
handle.get_stream());

Expand Down
6 changes: 5 additions & 1 deletion python/cugraph/cugraph/tests/link_analysis/test_hits.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ def setup_function():
fixture_params = gen_fixture_params_product(
(datasets, "graph_file"),
([50], "max_iter"),
([1.0e-6], "tol"),
# FIXME: Changed this from 1.0e-6 to 1.0e-5. NX defaults to
# FLOAT64 computation, cuGraph C++ defaults to whatever the edge weight
# is, cugraph python defaults that to FLOAT32. Does not converge at
# 1e-6 for larger graphs and FLOAT32.
([1.0e-5], "tol"),
)


Expand Down