Fix eigenvector testing and HITS testing discrepancies (#3979)

Address a couple of bugs discovered in nx-cugraph testing.

HITS should raise an exception if it fails to converge.

Eigenvector computation was not quite right, nx-cugraph testing discovered an edge condition where it was obvious.

Closes #3971
Closes #3972

Authors:
  - Chuck Hastings (https://github.com/ChuckHastings)

Approvers:
  - Naim (https://github.com/naimnv)
  - Seunghwa Kang (https://github.com/seunghwak)
  - Rick Ratzel (https://github.com/rlratzel)

URL: #3979

cpp/src/centrality/eigenvector_centrality_impl.cuh

@@ -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(
@@ -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,

cpp/src/link_analysis/hits_impl.cuh

@@ -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,
@@ -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) {
@@ -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

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;
 }

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};

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<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());
 

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"),
 )