graph: Extract Build() from the BaseGraph class.

* Add a BaseGraph::Builder class that simply forwards mutable calls (AddNode AddArc, Reserve, Build) to the underlying class.
* Update clients that use XXXStaticGraph, or a template graph to use Builder. Users who directly use ListGraph do not need to be changed if they do not call Build. If they do, remove the unnecessery call to Build.
* Move mutable functions from BaseGraph to XXXGraph::Builder.

Author: Mizux

ortools/algorithms/find_graph_symmetries_test.cc

@@ -696,22 +696,20 @@ TEST_F(FindSymmetriesTest, InwardGrid) {
   }
 }
 
-void AddReverseArcs(Graph* graph) {
+void AddReverseArcs(Graph::Builder& builder) {
+  const auto graph = Graph::Builder(builder).Build(nullptr);
   const int num_arcs = graph->num_arcs();
   for (int a = 0; a < num_arcs; ++a) {
-    graph->AddArc(graph->Head(a), graph->Tail(a));
+    builder.AddArc(graph->Head(a), graph->Tail(a));
   }
 }
 
-void AddReverseArcsAndFinalize(Graph* graph) {
-  AddReverseArcs(graph);
-  graph->Build();
-}
-
-void SetGraphEdges(absl::Span<const std::pair<int, int>> edges, Graph* graph) {
-  DCHECK_EQ(graph->num_arcs(), 0);
-  for (const auto [from, to] : edges) graph->AddArc(from, to);
-  AddReverseArcsAndFinalize(graph);
+std::unique_ptr<Graph> MakeGraphFromEdges(
+    absl::Span<const std::pair<int, int>> edges) {
+  Graph::Builder builder;
+  for (const auto [from, to] : edges) builder.AddArc(from, to);
+  AddReverseArcs(builder);
+  return std::move(builder).Build(nullptr);
 }
 
 TEST(CountTrianglesTest, EmptyGraph) {
@@ -723,19 +721,18 @@ TEST(CountTrianglesTest, SimpleUndirectedExample) {
   // 0--1--2
   //  `.|`.|
   //    3--4--5
-  Graph g;
-  SetGraphEdges(
-      {{0, 1}, {1, 2}, {0, 3}, {1, 4}, {1, 3}, {2, 4}, {3, 4}, {4, 5}}, &g);
+  const auto g = MakeGraphFromEdges(
+      {{0, 1}, {1, 2}, {0, 3}, {1, 4}, {1, 3}, {2, 4}, {3, 4}, {4, 5}});
   // Reminder: every undirected triangle counts as two directed triangles.
-  EXPECT_THAT(CountTriangles(g, /*max_degree=*/999),
+  EXPECT_THAT(CountTriangles(*g, /*max_degree=*/999),
               ElementsAre(2, 6, 2, 4, 4, 0));
-  EXPECT_THAT(CountTriangles(g, /*max_degree=*/3),
+  EXPECT_THAT(CountTriangles(*g, /*max_degree=*/3),
               ElementsAre(2, 0, 2, 4, 0, 0));
-  EXPECT_THAT(CountTriangles(g, /*max_degree=*/2),
+  EXPECT_THAT(CountTriangles(*g, /*max_degree=*/2),
               ElementsAre(2, 0, 2, 0, 0, 0));
-  EXPECT_THAT(CountTriangles(g, /*max_degree=*/1),
+  EXPECT_THAT(CountTriangles(*g, /*max_degree=*/1),
               ElementsAre(0, 0, 0, 0, 0, 0));
-  EXPECT_THAT(CountTriangles(g, /*max_degree=*/0),
+  EXPECT_THAT(CountTriangles(*g, /*max_degree=*/0),
               ElementsAre(0, 0, 0, 0, 0, 0));
 }
 
@@ -745,7 +742,7 @@ TEST(CountTrianglesTest, SimpleDirectedExample) {
   // 0     |    |     5
   //  \    |    v    /
   //   `-> 3 <- 4 <-'
-  Graph g;
+  Graph::Builder builder;
   for (auto [from, to] : std::vector<std::pair<int, int>>{
            {0, 1},
            {1, 2},
@@ -757,28 +754,27 @@ TEST(CountTrianglesTest, SimpleDirectedExample) {
            {2, 4},
            {4, 2},
        }) {
-    g.AddArc(from, to);
+    builder.AddArc(from, to);
   }
-  g.Build();
-  EXPECT_THAT(CountTriangles(g, /*max_degree=*/999),
+  const auto g = std::move(builder).Build(nullptr);
+  EXPECT_THAT(CountTriangles(*g, /*max_degree=*/999),
               ElementsAre(1, 0, 0, 0, 0, 2));
-  EXPECT_THAT(CountTriangles(g, /*max_degree=*/1),
+  EXPECT_THAT(CountTriangles(*g, /*max_degree=*/1),
               ElementsAre(0, 0, 0, 0, 0, 0));
 }
 
 TEST(LocalBfsTest, SimpleExample) {
   // 0--1--2
   //  `.|`.|
   //    3--4--5
-  Graph g;
-  SetGraphEdges(
-      {{0, 1}, {1, 2}, {0, 3}, {1, 4}, {1, 3}, {2, 4}, {3, 4}, {4, 5}}, &g);
-  std::vector<bool> tmp_mask(g.num_nodes(), false);
+  const auto g = MakeGraphFromEdges(
+      {{0, 1}, {1, 2}, {0, 3}, {1, 4}, {1, 3}, {2, 4}, {3, 4}, {4, 5}});
+  std::vector<bool> tmp_mask(g->num_nodes(), false);
   std::vector<int> visited;
   std::vector<int> num_within_radius;
 
   // Run a first unlimited BFS from 0.
-  LocalBfs(g, /*source=*/0, /*stop_after_num_nodes=*/99, &visited,
+  LocalBfs(*g, /*source=*/0, /*stop_after_num_nodes=*/99, &visited,
            &num_within_radius, &tmp_mask);
   EXPECT_THAT(
       visited,
@@ -791,28 +787,28 @@ TEST(LocalBfsTest, SimpleExample) {
 
   // Then a BFS that stops after visiting 4 nodes: we should finish exploring
   // that distance, i.e. explore 2 and 4, but not 5. Still, 5 is "visited".
-  LocalBfs(g, /*source=*/0, /*stop_after_num_nodes=*/4, &visited,
+  LocalBfs(*g, /*source=*/0, /*stop_after_num_nodes=*/4, &visited,
            &num_within_radius, &tmp_mask);
   EXPECT_THAT(visited, AnyOf(ElementsAre(0, 1, 3, 2, 4, 5),
                              ElementsAre(0, 1, 3, 4, 2, 5),
                              ElementsAre(0, 3, 1, 4, 2, 5)));
   EXPECT_THAT(num_within_radius, ElementsAre(1, 3, 5, 6));
 
   // Then a BFS that stops after visiting 2 nodes.
-  LocalBfs(g, /*source=*/0, /*stop_after_num_nodes=*/2, &visited,
+  LocalBfs(*g, /*source=*/0, /*stop_after_num_nodes=*/2, &visited,
            &num_within_radius, &tmp_mask);
   EXPECT_THAT(visited,
               AnyOf(ElementsAre(0, 1, 3, 2, 4), ElementsAre(0, 1, 3, 4, 2),
                     ElementsAre(0, 3, 1, 4, 2)));
   EXPECT_THAT(num_within_radius, ElementsAre(1, 3, 5));
 
   // Now run a BFS from node 3, stop exploring after 1 node.
-  LocalBfs(g, /*source=*/3, /*stop_after_num_nodes=*/1, &visited,
+  LocalBfs(*g, /*source=*/3, /*stop_after_num_nodes=*/1, &visited,
            &num_within_radius, &tmp_mask);
   EXPECT_THAT(visited, UnorderedElementsAre(3, 0, 1, 4));
   EXPECT_THAT(num_within_radius, ElementsAre(1, 4));
   // Now after 2 nodes.
-  LocalBfs(g, /*source=*/3, /*stop_after_num_nodes=*/2, &visited,
+  LocalBfs(*g, /*source=*/3, /*stop_after_num_nodes=*/2, &visited,
            &num_within_radius, &tmp_mask);
   EXPECT_THAT(visited, UnorderedElementsAre(3, 0, 1, 4, 2, 5));
   EXPECT_THAT(num_within_radius, ElementsAre(1, 4, 6));

ortools/bop/boolean_problem.cc

@@ -561,7 +561,7 @@ class IdGenerator {
 // in [0, num_classes) and any symmetry will only map nodes with the same class
 // between each other.
 template <typename Graph>
-Graph* GenerateGraphForSymmetryDetection(
+std::unique_ptr<const Graph> GenerateGraphForSymmetryDetection(
     const LinearBooleanProblem& problem,
     std::vector<int>* initial_equivalence_classes) {
   // First, we convert the problem to its canonical representation.
@@ -578,7 +578,7 @@ Graph* GenerateGraphForSymmetryDetection(
 
   // TODO(user): reserve the memory for the graph? not sure it is worthwhile
   // since it would require some linear scan of the problem though.
-  Graph* graph = new Graph();
+  typename Graph::Builder builder;
   initial_equivalence_classes->clear();
 
   // We will construct a graph with 3 different types of node that must be
@@ -593,8 +593,8 @@ Graph* GenerateGraphForSymmetryDetection(
     // Note that the indices are in [0, 2 * num_variables) and in one to one
     // correspondence with the index representation of a literal.
     const Literal literal = Literal(BooleanVariable(i), true);
-    graph->AddArc(literal.Index().value(), literal.NegatedIndex().value());
-    graph->AddArc(literal.NegatedIndex().value(), literal.Index().value());
+    builder.AddArc(literal.Index().value(), literal.NegatedIndex().value());
+    builder.AddArc(literal.NegatedIndex().value(), literal.Index().value());
   }
 
   // We use 0 for their initial equivalence class, but that may be modified
@@ -648,18 +648,18 @@ Graph* GenerateGraphForSymmetryDetection(
         // Connect this node to the constraint node. Note that we don't
         // technically need the arcs in both directions, but that may help a bit
         // the algorithm to find symmetries.
-        graph->AddArc(constraint_node_index, current_node_index);
-        graph->AddArc(current_node_index, constraint_node_index);
+        builder.AddArc(constraint_node_index, current_node_index);
+        builder.AddArc(current_node_index, constraint_node_index);
       }
 
       // Connect this node to the associated term.literal node. Note that we
       // don't technically need the arcs in both directions, but that may help a
       // bit the algorithm to find symmetries.
-      graph->AddArc(current_node_index, term.literal.Index().value());
-      graph->AddArc(term.literal.Index().value(), current_node_index);
+      builder.AddArc(current_node_index, term.literal.Index().value());
+      builder.AddArc(term.literal.Index().value(), current_node_index);
     }
   }
-  graph->Build();
+  auto graph = std::move(builder).Build(nullptr);
   DCHECK_EQ(graph->num_nodes(), initial_equivalence_classes->size());
   return graph;
 }
@@ -704,8 +704,8 @@ void FindLinearBooleanProblemSymmetries(
     std::vector<std::unique_ptr<SparsePermutation>>* generators) {
   typedef GraphSymmetryFinder::Graph Graph;
   std::vector<int> equivalence_classes;
-  std::unique_ptr<Graph> graph(
-      GenerateGraphForSymmetryDetection<Graph>(problem, &equivalence_classes));
+  std::unique_ptr<const Graph> graph =
+      GenerateGraphForSymmetryDetection<Graph>(problem, &equivalence_classes);
   LOG(INFO) << "Graph has " << graph->num_nodes() << " nodes and "
             << graph->num_arcs() / 2 << " edges.";
 #if defined(ORTOOLS_TARGET_OS_SUPPORTS_FILE)

ortools/graph/BUILD.bazel

@@ -48,12 +48,9 @@ cc_test(
         "//ortools/base:types",
         "//ortools/graph_base:graph",
         "//ortools/graph_base:io",
-        "//ortools/graph_base:test_util",
         "//ortools/util:flat_matrix",
-        "@abseil-cpp//absl/log:check",
         "@abseil-cpp//absl/random",
         "@abseil-cpp//absl/random:distributions",
-        "@google_benchmark//:benchmark",
     ],
 )
 
@@ -150,7 +147,6 @@ cc_test(
         "@abseil-cpp//absl/random:distributions",
         "@abseil-cpp//absl/strings",
         "@abseil-cpp//absl/types:span",
-        "@google_benchmark//:benchmark",
     ],
 )
 
@@ -215,7 +211,6 @@ cc_test(
         "@abseil-cpp//absl/status",
         "@abseil-cpp//absl/strings:str_format",
         "@abseil-cpp//absl/types:span",
-        "@google_benchmark//:benchmark",
     ],
 )
 
@@ -234,7 +229,6 @@ cc_test(
         "//ortools/base:gmock_main",
         "//ortools/graph_base:graph",
         "@abseil-cpp//absl/base:core_headers",
-        "@google_benchmark//:benchmark",
     ],
 )
 
@@ -258,7 +252,6 @@ cc_test(
         "//ortools/base:types",
         "//ortools/graph_base:graph",
         "@abseil-cpp//absl/base:core_headers",
-        "@abseil-cpp//absl/random:distributions",
         "@abseil-cpp//absl/types:span",
         "@google_benchmark//:benchmark",
     ],
@@ -473,7 +466,6 @@ cc_test(
         "@abseil-cpp//absl/random:distributions",
         "@abseil-cpp//absl/strings:str_format",
         "@abseil-cpp//absl/types:span",
-        "@google_benchmark//:benchmark",
     ],
 )
 
@@ -558,7 +550,6 @@ cc_test(
         "//ortools/graph_base:graph",
         "@abseil-cpp//absl/flags:flag",
         "@abseil-cpp//absl/log:check",
-        "@abseil-cpp//absl/random:distributions",
         "@abseil-cpp//absl/types:span",
         "@google_benchmark//:benchmark",
     ],
@@ -690,10 +681,7 @@ cc_test(
         "//ortools/base:gmock_main",
         "//ortools/graph_base:graph",
         "@abseil-cpp//absl/algorithm:container",
-        "@abseil-cpp//absl/log:check",
-        "@abseil-cpp//absl/random",
         "@abseil-cpp//absl/status",
-        "@google_benchmark//:benchmark",
     ],
 )
 
@@ -715,7 +703,6 @@ cc_test(
         ":minimum_vertex_cover",
         "//ortools/base:gmock_main",
         "@abseil-cpp//absl/algorithm:container",
-        "@google_benchmark//:benchmark",
     ],
 )
 
@@ -754,6 +741,5 @@ cc_test(
         "@abseil-cpp//absl/random",
         "@abseil-cpp//absl/status",
         "@abseil-cpp//absl/types:span",
-        "@google_benchmark//:benchmark",
     ],
 )

ortools/graph/README.md

@@ -158,8 +158,8 @@ node.
     memory loads.
 
 -   `StaticGraph<>`: More memory and speed efficient than `ListGraph<>`, but
-    requires calling `Build()` once after adding all nodes and arcs. Iterating
-    outgoing arcs requires only `2` memory loads.
+    requires building after adding all nodes and arcs. Iterating outgoing arcs
+    requires only `2` memory loads.
 
 -   `ReverseArcListGraph<>` adds reverse arcs to `ListGraph<>`. Iterating
     neighboring arcs for a node requires `O(degree)` memory loads.
@@ -202,18 +202,18 @@ You can find some canonical examples in [`samples`][samples].
 
 <!-- Links used throughout the document. -->
 [graph_h]: ../graph/graph.h
-[bounded_dijkstra_h]: http://google3/third_party/ortools/ortools/graph/bounded_dijkstra.h
-[bidirectional_dijkstra_h]: http://google3/third_party/ortools/ortools/graph/bidirectional_dijkstra.h
-[shortest_paths_h]: http://google3/third_party/ortools/ortools/graph/shortest_paths.h
-[dag_shortest_path_h]: http://google3/third_party/ortools/ortools/graph/dag_shortest_path.h
-[dag_constrained_shortest_path_h]: http://google3/third_party/ortools/ortools/graph/dag_constrained_shortest_path.h
-[hamiltonian_path_h]: http://google3/third_party/ortools/ortools/graph/hamiltonian_path.h
-[eulerian_path_h]: http://google3/third_party/ortools/ortools/graph/eulerian_path.h
-[connected_components_h]: http://google3/third_party/ortools/ortools/graph/connected_components.h
-[strongly_connected_components_h]: http://google3/third_party/ortools/ortools/graph/strongly_connected_components.h
-[cliques_h]: http://google3/third_party/ortools/ortools/graph/cliques.h
-[linear_assignment_h]: http://google3/third_party/ortools/ortools/graph/linear_assignment.h
-[max_flow_h]: http://google3/third_party/ortools/ortools/graph/max_flow.h
-[min_cost_flow_h]: http://google3/third_party/ortools/ortools/graph/min_cost_flow.h
-[samples]: http://google3/third_party/ortools/ortools/graph/samples/
+[bounded_dijkstra_h]: ../graph/bounded_dijkstra.h
+[bidirectional_dijkstra_h]: ../graph/bidirectional_dijkstra.h
+[shortest_paths_h]: ../graph/shortest_paths.h
+[dag_shortest_path_h]: ../graph/dag_shortest_path.h
+[dag_constrained_shortest_path_h]: ../graph/dag_constrained_shortest_path.h
+[hamiltonian_path_h]: ../graph/hamiltonian_path.h
+[eulerian_path_h]: ../graph/eulerian_path.h
+[connected_components_h]: ../graph/connected_components.h
+[strongly_connected_components_h]: ../graph/strongly_connected_components.h
+[cliques_h]: ../graph/cliques.h
+[linear_assignment_h]: ../graph/linear_assignment.h
+[max_flow_h]: ../graph/max_flow.h
+[min_cost_flow_h]: ../graph/min_cost_flow.h
+[samples]: ../graph/samples/
 [graph]: ../graph/

ortools/graph/assignment.cc

@@ -65,6 +65,7 @@ SimpleLinearSumAssignment::Status SimpleLinearSumAssignment::Solve() {
   optimal_cost_ = 0;
   assignment_arcs_.clear();
   if (NumNodes() == 0) return OPTIMAL;
+
   // HACK(user): Detect overflows early. In ./linear_assignment.h, the cost of
   // each arc is internally multiplied by cost_scaling_factor_ (which is equal
   // to (num_nodes + 1)) without overflow checking.
@@ -74,6 +75,11 @@ SimpleLinearSumAssignment::Status SimpleLinearSumAssignment::Solve() {
     if (unscaled_arc_cost > max_supported_arc_cost) return POSSIBLE_OVERFLOW;
   }
 
+  // Fast path if the graph is complete.
+  if (const auto solve_satus = SolveIfCompleteBipartite()) {
+    return *solve_satus;
+  }
+
   const ArcIndex num_arcs = arc_cost_.size();
   ::util::ListGraph<> graph(2 * num_nodes_, num_arcs);
   LinearSumAssignment<::util::ListGraph<>, CostValue> assignment(graph,
@@ -93,4 +99,40 @@ SimpleLinearSumAssignment::Status SimpleLinearSumAssignment::Solve() {
   return OPTIMAL;
 }
 
+std::optional<SimpleLinearSumAssignment::Status>
+SimpleLinearSumAssignment::SolveIfCompleteBipartite() {
+  // Quick check: only do something if we have the correct number of arcs.
+  const ArcIndex num_arcs = arc_cost_.size();
+  if (static_cast<int64_t>(num_arcs) !=
+      static_cast<int64_t>(num_nodes_) * num_nodes_) {
+    return std::nullopt;
+  }
+
+  // If there are no duplicate arcs, this is a complete bipartite graph!
+  // Otherwise, we will fall back to the general case.
+  ::util::CompleteBipartiteGraph<> graph(num_nodes_, num_nodes_);
+  std::vector<int> reverse_mapping(num_arcs, -1);
+  LinearSumAssignment<::util::CompleteBipartiteGraph<>, CostValue> assignment(
+      graph, num_nodes_);
+
+  for (ArcIndex user_index = 0; user_index < num_arcs; ++user_index) {
+    const ::util::CompleteBipartiteGraph<>::ArcIndex new_index =
+        graph.GetArc(arc_tail_[user_index], num_nodes_ + arc_head_[user_index]);
+
+    // Abort if we have duplicate arcs.
+    if (reverse_mapping[new_index] != -1) return std::nullopt;
+    reverse_mapping[new_index] = user_index;
+    assignment.SetArcCost(new_index, arc_cost_[user_index]);
+  }
+
+  if (!assignment.FinalizeSetup()) return POSSIBLE_OVERFLOW;
+  if (!assignment.ComputeAssignment()) return INFEASIBLE;
+  optimal_cost_ = assignment.GetCost();
+  for (NodeIndex node = 0; node < num_nodes_; ++node) {
+    assignment_arcs_.push_back(
+        reverse_mapping[assignment.GetAssignmentArc(node)]);
+  }
+  return OPTIMAL;
+}
+
 }  // namespace operations_research