ligra: Add comments and unit test regarding bug with vertex intersection

ligra/BUILD

@@ -128,6 +128,8 @@ cc_library(
   ":macros",
   ":undirected_edge",
   ":vertex",
+  "//pbbslib:assert",
+  "//pbbslib:sample_sort",
   "//pbbslib:utilities",
   ]
 )

ligra/graph_test_utils.cc

@@ -2,14 +2,19 @@
 
 #include <tuple>
 
+#include "pbbslib/assert.h"
+#include "pbbslib/sample_sort.h"
+
 namespace graph_test {
 
 symmetric_graph<symmetric_vertex, pbbslib::empty> MakeUnweightedSymmetricGraph(
     const uintE num_vertices,
-    const std::unordered_set<UndirectedEdge>& edges) {
+    const std::unordered_set<UndirectedEdge>& edges,
+    const ShouldSortNeighbors should_sort_neighbors) {
+  using Edge = std::tuple<uintE, uintE, pbbslib::empty>;
   constexpr pbbs::empty weight{};
-  pbbs::sequence<std::tuple<uintE, uintE, pbbslib::empty>> edge_sequence(
-      edges.size() * 2);
+
+  pbbs::sequence<Edge> edge_sequence(edges.size() * 2);
   auto edges_it{edges.cbegin()};
   for (size_t i = 0; i < edges.size(); i++) {
     edge_sequence[2 * i] =
@@ -24,7 +29,23 @@ symmetric_graph<symmetric_vertex, pbbslib::empty> MakeUnweightedSymmetricGraph(
           weight);
     ++edges_it;
   }
-  return sym_graph_from_edges(edge_sequence, num_vertices);
+
+  switch (should_sort_neighbors) {
+    case ShouldSortNeighbors::kYes: {
+      pbbs::sample_sort_inplace(
+          edge_sequence.slice(),
+          [](const Edge& left, const Edge& right) {
+            return std::tie(std::get<0>(left), std::get<1>(left))
+              < std::tie(std::get<0>(right), std::get<1>(right));
+          });
+      constexpr bool kEdgesAreSorted{true};
+      return sym_graph_from_edges(edge_sequence, num_vertices, kEdgesAreSorted);
+    }
+    case ShouldSortNeighbors::kNo: {
+      return sym_graph_from_edges(edge_sequence, num_vertices);
+    }
+  }
+  ABORT_INVALID_ENUM(ShouldSortNeighbors, should_sort_neighbors);
 }
 
 }  // namespace graph_test

ligra/graph_test_utils.h

@@ -12,9 +12,15 @@
 
 namespace graph_test {
 
+enum class ShouldSortNeighbors { kYes, kNo };
+
 // Make an undirected, unweighted graph from a list of edges.
+//
+// If `should_sort_neighbors` is set to `ShouldSortNeighbors::kYes`, then each
+// vertex's list of neighbors will be sorted in the graph representation.
 symmetric_graph<symmetric_vertex, pbbslib::empty> MakeUnweightedSymmetricGraph(
     const uintE num_vertices,
-    const std::unordered_set<UndirectedEdge>& edges);
+    const std::unordered_set<UndirectedEdge>& edges,
+    ShouldSortNeighbors should_sort_neighbors = ShouldSortNeighbors::kNo);
 
 }  // namespace graph_test

ligra/unit_tests/BUILD

@@ -3,6 +3,7 @@ cc_test(
     srcs = ["graph_test.cc"],
     deps = [
         "//ligra:graph",
+        "//ligra:graph_test_utils",
         "//pbbslib:seq",
         "@googletest//:gtest_main",
     ],

ligra/unit_tests/graph_test.cc

@@ -1,7 +1,10 @@
 #include <gtest/gtest.h>
 #include "ligra/graph.h"
+#include "ligra/graph_test_utils.h"
 #include "pbbslib/seq.h"
 
+namespace gt = graph_test;
+
 TEST(TestSymGraphFromEdges, TestBrokenPath) {
   using edge = std::tuple<uintE, uintE, int>;
   uintE n = 11;
@@ -49,3 +52,50 @@ TEST(TestSymGraphFromEdges, TestGraphWithSingletons) {
   ASSERT_EQ(graph.get_vertex(2).getOutDegree(), 0);
   ASSERT_EQ(graph.get_vertex(3).getOutDegree(), 0);
 }
+
+TEST(symmetric_vertex, Intersect) {
+  using Vertex = symmetric_vertex<pbbs::empty>;
+
+  // Graph diagram:
+  //   0 - 1 - 2
+  //    \ / \ /
+  //     3 - 4 -- 5
+  constexpr uintE kNumVertices{6};
+  const std::unordered_set<UndirectedEdge> kEdges{
+    {0, 1},
+    {0, 3},
+    {1, 2},
+    {1, 3},
+    {1, 4},
+    {2, 4},
+    {3, 4},
+    {4, 5},
+  };
+  auto graph{gt::MakeUnweightedSymmetricGraph(
+      kNumVertices, kEdges, gt::ShouldSortNeighbors::kYes)};
+
+  {
+    const uintE u_id{0};
+    const uintE v_id{5};
+    Vertex u{graph.get_vertex(u_id)};
+    Vertex v{graph.get_vertex(v_id)};
+    EXPECT_EQ((u.intersect(&v, u_id, v_id)), 0);
+    EXPECT_EQ((v.intersect(&u, v_id, u_id)), 0);
+  }
+  {
+    const uintE u_id{0};
+    const uintE v_id{1};
+    Vertex u{graph.get_vertex(u_id)};
+    Vertex v{graph.get_vertex(v_id)};
+    EXPECT_EQ((u.intersect(&v, u_id, v_id)), 1);
+    EXPECT_EQ((v.intersect(&u, v_id, u_id)), 1);
+  }
+  {
+    const uintE u_id{1};
+    const uintE v_id{3};
+    Vertex u{graph.get_vertex(u_id)};
+    Vertex v{graph.get_vertex(v_id)};
+    EXPECT_EQ((u.intersect(&v, u_id, v_id)), 2);
+    EXPECT_EQ((v.intersect(&u, v_id, u_id)), 2);
+  }
+}

ligra/vertex.h

@@ -26,6 +26,7 @@
 #include "pbbslib/sequence_ops.h"
 #include "macros.h"
 
+// This `intersection` namespace is intended for internal use only.
 namespace intersection {
 
 template <template <typename W> class vertex, class W>
@@ -487,17 +488,37 @@ struct symmetric_vertex {
     return vertex_ops::get_iter(getOutNeighbors(), getOutDegree());
   }
 
+  // Computes the number of neighbors that this vertex and vertex `other`
+  // shares.
+  //
+  // This function will only return correct results if the neighbor lists of
+  // `this` and `other` are both sorted in ascending order. This condition does
+  // not necessarily hold for all graphs.
+  //
+  // `our_id` must be the ID of `this`.
+  // `other` is the vertex to intersect with, and `other_id` must be its ID.
   inline size_t intersect(symmetric_vertex<W>* other, long our_id,
                           long other_id) {
     return intersection::intersect(this, other, our_id, other_id);
   }
 
+  // Same as `intersect`, but runs `f(our_id, other_id, shared_neighbor_id)` for
+  // each shared neighbor between the two vertices.
+  //
+  // This function will only return correct results if the neighbor lists of
+  // `this` and `other` are both sorted in ascending order. This condition does
+  // not necessarily hold for all graphs.
   template <class F>
   inline size_t intersect_f(symmetric_vertex<W>* other, long our_id,
                             long other_id, const F& f) {
     return intersection::intersect_f(this, other, our_id, other_id, f);
   }
 
+  // Parallel version of `intersect_f`.
+  //
+  // This function will only return correct results if the neighbor lists of
+  // `this` and `other` are both sorted in ascending order. This condition does
+  // not necessarily hold for all graphs.
   template <class F>
   inline size_t intersect_f_par(symmetric_vertex<W>* other, long our_id,
                             long other_id, const F& f) {
@@ -752,17 +773,20 @@ struct asymmetric_vertex {
     return vertex_ops::get_iter(getOutNeighbors(), getOutDegree());
   }
 
+  // See comment for `symmetric_vertex::intersect`.
   inline size_t intersect(asymmetric_vertex<W>* other, long our_id,
                           long other_id) {
     return intersection::intersect(this, other, our_id, other_id);
   }
 
+  // See comment for `symmetric_vertex::intersect_f`.
   template <class F>
   inline size_t intersect_f(asymmetric_vertex<W>* other, long our_id,
                             long other_id, const F& f) {
     return intersection::intersect_f(this, other, our_id, other_id, f);
   }
 
+  // See comment for `symmetric_vertex::intersect_f_par`.
   template <class F>
   inline size_t intersect_f_par(asymmetric_vertex<W>* other, long our_id,
                             long other_id, const F& f) {