From e0fa30e3d64b00d9ba4a6e5f66881fb23f1daa6a Mon Sep 17 00:00:00 2001
From: Tom Tseng <tom.hm.tseng@gmail.com>
Date: Thu, 5 Mar 2020 17:11:34 -0500
Subject: [PATCH 1/7] vertex: Add comments to intersect functions

---
 ligra/vertex.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/ligra/vertex.h b/ligra/vertex.h
index b3b127bd2..a41568602 100644
--- a/ligra/vertex.h
+++ b/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) {

From 86ceab500a685da1709c7eca14ef61814dc227e3 Mon Sep 17 00:00:00 2001
From: Tom Tseng <tom.hm.tseng@gmail.com>
Date: Thu, 5 Mar 2020 17:11:56 -0500
Subject: [PATCH 2/7] graph_test_utils: Add option to sort neighbors in making
 test graphs

---
 ligra/BUILD               |  1 +
 ligra/graph_test_utils.cc | 27 +++++++++++++++++++++++----
 ligra/graph_test_utils.h  |  8 +++++++-
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/ligra/BUILD b/ligra/BUILD
index 8868eccb7..0bae34509 100644
--- a/ligra/BUILD
+++ b/ligra/BUILD
@@ -127,6 +127,7 @@ cc_library(
   ":macros",
   ":undirected_edge",
   ":vertex",
+  "//pbbslib:sample_sort",
   "//pbbslib:utilities",
   ]
 )
diff --git a/ligra/graph_test_utils.cc b/ligra/graph_test_utils.cc
index baf4c60a6..02cc32a06 100644
--- a/ligra/graph_test_utils.cc
+++ b/ligra/graph_test_utils.cc
@@ -2,14 +2,18 @@
 
 #include <tuple>
 
+#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 +28,22 @@ 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));
+          });
+      const bool edges_are_sorted{true};
+      return sym_graph_from_edges(edge_sequence, num_vertices, edges_are_sorted);
+    }
+    case ShouldSortNeighbors::kNo: {
+      return sym_graph_from_edges(edge_sequence, num_vertices);
+    }
+  }
 }
 
 }  // namespace graph_test
diff --git a/ligra/graph_test_utils.h b/ligra/graph_test_utils.h
index 351a1dc80..1eb2ac5b6 100644
--- a/ligra/graph_test_utils.h
+++ b/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

From f0197483768cf02c1a6d28694a73d70f8c09b6a6 Mon Sep 17 00:00:00 2001
From: Tom Tseng <tom.hm.tseng@gmail.com>
Date: Thu, 5 Mar 2020 17:12:16 -0500
Subject: [PATCH 3/7] ligra: Add unit tests for vertex neighborhood
 intersection

---
 ligra/unit_tests/BUILD         |  1 +
 ligra/unit_tests/graph_test.cc | 50 ++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/ligra/unit_tests/BUILD b/ligra/unit_tests/BUILD
index aa01d970e..2445bb500 100644
--- a/ligra/unit_tests/BUILD
+++ b/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",
     ],
diff --git a/ligra/unit_tests/graph_test.cc b/ligra/unit_tests/graph_test.cc
index a2981962e..4ec2455d3 100644
--- a/ligra/unit_tests/graph_test.cc
+++ b/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;
@@ -32,3 +35,50 @@ TEST(TestSymGraphFromEdges, TestBrokenPath) {
   ASSERT_EQ(G.get_vertex(9).getOutDegree(), 2);
   ASSERT_EQ(G.get_vertex(10).getOutDegree(), 3);
 }
+
+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);
+  }
+}

From 3e07be016964b38d465e9dbd4ee3ff7263747b63 Mon Sep 17 00:00:00 2001
From: Tom Tseng <tom.hm.tseng@gmail.com>
Date: Thu, 5 Mar 2020 22:50:09 +0000
Subject: [PATCH 4/7] assert: Add assert function for handling complete enum
 switch statements

---
 pbbslib/assert.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/pbbslib/assert.h b/pbbslib/assert.h
index 479c1d33d..70d529ad4 100644
--- a/pbbslib/assert.h
+++ b/pbbslib/assert.h
@@ -25,6 +25,28 @@
     std::terminate(); \
   } while (false)
 
+// Prints out enum value and terminates the program.
+//
+// This is intended for use after a switch statement on an enum that handles all
+// possible enum cases by returning. Annoyingly, GCC can complain about this.
+// Example:
+//     enum class OneOrTwo { kOne, kTwo };
+//
+//     int ReturnOneOrTwo(OneOrTwo one_or_two) {
+//       switch (one_or_two) {
+//         case OneOrTwo::kOne:
+//           return 1;
+//         case OneOrTwo::kTwo:
+//           return 2;
+//       }
+//       // If we didn't put this ABORT statement below, GCC would complain:
+//       //   warning: control reaches end of non-void function [-Wreturn-type]
+//       ABORT_INVALID_ENUM(OneOrTwo, one_or_two)
+//     }
+#define ABORT_INVALID_ENUM(EnumType, enum_value) \
+  ABORT("Unexpected " #EnumType " value: " << \
+      static_cast<typename std::underlying_type<EnumType>::type>(enum_value));
+
 // Asserts on a condition, printing an error and terminating if the condition is
 // false.
 //

From 0673c9a96991e3dc16bb6e61e798e7b45b9d3e06 Mon Sep 17 00:00:00 2001
From: Tom Tseng <tom.hm.tseng@gmail.com>
Date: Thu, 5 Mar 2020 22:52:17 +0000
Subject: [PATCH 5/7] ligra: fix compilation warning for enum switch statement
 in unit test

---
 ligra/BUILD               | 1 +
 ligra/graph_test_utils.cc | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/ligra/BUILD b/ligra/BUILD
index 0bae34509..f80530e13 100644
--- a/ligra/BUILD
+++ b/ligra/BUILD
@@ -127,6 +127,7 @@ cc_library(
   ":macros",
   ":undirected_edge",
   ":vertex",
+  "//pbbslib:assert",
   "//pbbslib:sample_sort",
   "//pbbslib:utilities",
   ]
diff --git a/ligra/graph_test_utils.cc b/ligra/graph_test_utils.cc
index 02cc32a06..350dde4fe 100644
--- a/ligra/graph_test_utils.cc
+++ b/ligra/graph_test_utils.cc
@@ -2,6 +2,7 @@
 
 #include <tuple>
 
+#include "pbbslib/assert.h"
 #include "pbbslib/sample_sort.h"
 
 namespace graph_test {
@@ -44,6 +45,7 @@ symmetric_graph<symmetric_vertex, pbbslib::empty> MakeUnweightedSymmetricGraph(
       return sym_graph_from_edges(edge_sequence, num_vertices);
     }
   }
+  ABORT_INVALID_ENUM(ShouldSortNeighbors, should_sort_neighbors);
 }
 
 }  // namespace graph_test

From 6fb29b90d76e27e96beab7a16e556dd5693839ef Mon Sep 17 00:00:00 2001
From: Tom Tseng <tom.hm.tseng@gmail.com>
Date: Thu, 5 Mar 2020 18:07:44 -0500
Subject: [PATCH 6/7] assert: Clarify comments on ABORT_INVALID_ENUM

---
 pbbslib/assert.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/pbbslib/assert.h b/pbbslib/assert.h
index 70d529ad4..c0af3b206 100644
--- a/pbbslib/assert.h
+++ b/pbbslib/assert.h
@@ -28,7 +28,8 @@
 // Prints out enum value and terminates the program.
 //
 // This is intended for use after a switch statement on an enum that handles all
-// possible enum cases by returning. Annoyingly, GCC can complain about this.
+// possible enum cases by returning. Using this macro will stop GCC's
+// -Wreturn-type flag from issuing a warning about this situation.
 // Example:
 //     enum class OneOrTwo { kOne, kTwo };
 //
@@ -41,6 +42,8 @@
 //       }
 //       // If we didn't put this ABORT statement below, GCC would complain:
 //       //   warning: control reaches end of non-void function [-Wreturn-type]
+//       // We shouldn't reach this point unless someone makes a weird function
+//       // call like `ReturnOneOrTwo(static_cast<OneOrTwo>(3))`.
 //       ABORT_INVALID_ENUM(OneOrTwo, one_or_two)
 //     }
 #define ABORT_INVALID_ENUM(EnumType, enum_value) \

From 5c61adf8cb0d219eaecfccaf5af1e637b5e57386 Mon Sep 17 00:00:00 2001
From: Tom Tseng <tom.hm.tseng@gmail.com>
Date: Sat, 7 Mar 2020 19:22:25 -0500
Subject: [PATCH 7/7] graph_test_utils: variable naming style nit

---
 ligra/graph_test_utils.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ligra/graph_test_utils.cc b/ligra/graph_test_utils.cc
index 350dde4fe..c4f1a7fae 100644
--- a/ligra/graph_test_utils.cc
+++ b/ligra/graph_test_utils.cc
@@ -38,8 +38,8 @@ symmetric_graph<symmetric_vertex, pbbslib::empty> MakeUnweightedSymmetricGraph(
             return std::tie(std::get<0>(left), std::get<1>(left))
               < std::tie(std::get<0>(right), std::get<1>(right));
           });
-      const bool edges_are_sorted{true};
-      return sym_graph_from_edges(edge_sequence, num_vertices, edges_are_sorted);
+      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);