diff --git a/cpp/src/centrality/eigenvector_centrality_impl.cuh b/cpp/src/centrality/eigenvector_centrality_impl.cuh index 291abf18455..8d1bea4004d 100644 --- a/cpp/src/centrality/eigenvector_centrality_impl.cuh +++ b/cpp/src/centrality/eigenvector_centrality_impl.cuh @@ -96,7 +96,8 @@ rmm::device_uvector 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( @@ -122,6 +123,13 @@ rmm::device_uvector eigenvector_centrality( centralities.begin()); } + thrust::transform(handle.get_thrust_policy(), + centralities.begin(), + centralities.end(), + old_centralities.begin(), + centralities.begin(), + thrust::plus()); + // Normalize the centralities auto hypotenuse = sqrt(transform_reduce_v( handle, diff --git a/cpp/src/link_analysis/hits_impl.cuh b/cpp/src/link_analysis/hits_impl.cuh index 9badb041218..674046745b1 100644 --- a/cpp/src/link_analysis/hits_impl.cuh +++ b/cpp/src/link_analysis/hits_impl.cuh @@ -112,7 +112,8 @@ std::tuple 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, @@ -162,17 +163,19 @@ std::tuple 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) { @@ -188,7 +191,7 @@ std::tuple 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 diff --git a/cpp/tests/c_api/eigenvector_centrality_test.c b/cpp/tests/c_api/eigenvector_centrality_test.c index 9fd2d2bee6f..8bc5971a70c 100644 --- a/cpp/tests/c_api/eigenvector_centrality_test.c +++ b/cpp/tests/c_api/eigenvector_centrality_test.c @@ -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; } diff --git a/cpp/tests/centrality/eigenvector_centrality_test.cpp b/cpp/tests/centrality/eigenvector_centrality_test.cpp index 7cafcfbde85..6c3bd510abd 100644 --- a/cpp/tests/centrality/eigenvector_centrality_test.cpp +++ b/cpp/tests/centrality/eigenvector_centrality_test.cpp @@ -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}; diff --git a/cpp/tests/link_analysis/hits_test.cpp b/cpp/tests/link_analysis/hits_test.cpp index 44fa619b503..d0e77769034 100644 --- a/cpp/tests/link_analysis/hits_test.cpp +++ b/cpp/tests/link_analysis/hits_test.cpp @@ -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. @@ -176,7 +176,7 @@ class Tests_Hits : public ::testing::TestWithParam d_hubs(graph_view.local_vertex_partition_range_size(), handle.get_stream()); diff --git a/python/cugraph/cugraph/tests/link_analysis/test_hits.py b/python/cugraph/cugraph/tests/link_analysis/test_hits.py index 1c5a135e944..fcfd8cc5318 100644 --- a/python/cugraph/cugraph/tests/link_analysis/test_hits.py +++ b/python/cugraph/cugraph/tests/link_analysis/test_hits.py @@ -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"), )