minibmg: change std::list to std::vector (#1792)

Summary:
Pull Request resolved: #1792

We change all uses of `std::list` to `std::vector` throughout minibmg.  The books I've read recommond not using `list` unless you really need some of its APIs, which we do not.  `vector` is faster and uses less memory.

I'm doing this now because I need it in another diff, and I want to keep each diff as simple as possible.

Reviewed By: AishwaryaSivaraman

Differential Revision: D40812419

fbshipit-source-id: 0ce97a5f3570aa2170e1ec48c7e285f15eb35fdb

minibmg/ad/reverse.h

@@ -49,12 +49,14 @@ requires Number<Underlying>
 class ReverseBody {
  public:
   Underlying primal;
-  std::list<Reverse<Underlying>> inputs;
+  std::vector<Reverse<Underlying>> inputs;
   Underlying adjoint = 0;
 
   /* implicit */ ReverseBody(double primal);
   /* implicit */ ReverseBody(Underlying primal);
-  ReverseBody(Underlying primal, const std::list<Reverse<Underlying>>& inputs);
+  ReverseBody(
+      Underlying primal,
+      const std::vector<Reverse<Underlying>>& inputs);
   ReverseBody(const ReverseBody<Underlying>& other);
   ReverseBody<Underlying>& operator=(const ReverseBody<Underlying>& other);
   virtual ~ReverseBody() {}
@@ -95,17 +97,17 @@ requires Number<Underlying> Reverse<Underlying>
 template <class Underlying>
 class PrecomputedGradients : public ReverseBody<Underlying> {
  public:
-  const std::list<Underlying> computed_partial_derivatives;
+  const std::vector<Underlying> computed_partial_derivatives;
   PrecomputedGradients(
       Underlying primal,
-      const std::list<Reverse<Underlying>>& inputs,
-      const std::list<Underlying>& computed_partial_derivatives)
+      const std::vector<Reverse<Underlying>>& inputs,
+      const std::vector<Underlying>& computed_partial_derivatives)
       : ReverseBody<Underlying>{primal, inputs},
         computed_partial_derivatives{computed_partial_derivatives} {}
   void backprop() override {
-    auto& /*std::list<Reverse<Underlying>>*/ t = this->inputs;
-    typename std::list<Reverse<Underlying>>::iterator it1 = t.begin();
-    typename std::list<Underlying>::const_iterator it2 =
+    auto& /*std::vector<Reverse<Underlying>>*/ t = this->inputs;
+    typename std::vector<Reverse<Underlying>>::iterator it1 = t.begin();
+    typename std::vector<Underlying>::const_iterator it2 =
         computed_partial_derivatives.begin();
     for (; it1 != t.end() && it2 != computed_partial_derivatives.end();
          ++it1, ++it2) {
@@ -132,7 +134,7 @@ requires Number<Underlying>
 void Reverse<Underlying>::reverse(double initial_adjoint) {
   // topologically sort the set of ReverseBody pointers reachable - these are
   // the nodes to which we need to backprop.
-  std::list<Bodyp> roots = {ptr};
+  std::vector<Bodyp> roots = {ptr};
   std::function<std::vector<Bodyp>(const Bodyp&)> predecessors =
       [&](const Bodyp& ptr) -> std::vector<Bodyp> {
     std::vector<Bodyp> result;
@@ -172,7 +174,7 @@ requires Number<Underlying> ReverseBody<Underlying>::ReverseBody(
 template <class Underlying>
 requires Number<Underlying> ReverseBody<Underlying>::ReverseBody(
     Underlying primal,
-    const std::list<Reverse<Underlying>>& inputs)
+    const std::vector<Reverse<Underlying>>& inputs)
     : primal{primal}, inputs{inputs} {}
 
 template <class Underlying>
@@ -185,35 +187,35 @@ operator+(const Reverse<Underlying>& left, const Reverse<Underlying>& right) {
   Underlying new_primal = left.ptr->primal + right.ptr->primal;
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       new_primal,
-      std::list<Reverse<Underlying>>{left, right},
-      std::list<Underlying>{1, 1})};
+      std::vector<Reverse<Underlying>>{left, right},
+      std::vector<Underlying>{1, 1})};
 }
 
 template <class Underlying>
 requires Number<Underlying> Reverse<Underlying>
 operator-(const Reverse<Underlying>& left, const Reverse<Underlying>& right) {
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       left.ptr->primal - right.ptr->primal,
-      std::list<Reverse<Underlying>>{left, right},
-      std::list<Underlying>{1, -1})};
+      std::vector<Reverse<Underlying>>{left, right},
+      std::vector<Underlying>{1, -1})};
 }
 
 template <class Underlying>
 requires Number<Underlying> Reverse<Underlying>
 operator-(const Reverse<Underlying>& x) {
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       -x.ptr->primal,
-      std::list<Reverse<Underlying>>{x},
-      std::list<Underlying>{-1})};
+      std::vector<Reverse<Underlying>>{x},
+      std::vector<Underlying>{-1})};
 }
 
 template <class Underlying>
 requires Number<Underlying> Reverse<Underlying>
 operator*(const Reverse<Underlying>& left, const Reverse<Underlying>& right) {
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       left.ptr->primal * right.ptr->primal,
-      std::list<Reverse<Underlying>>{left, right},
-      std::list<Underlying>{right.ptr->primal, left.ptr->primal})};
+      std::vector<Reverse<Underlying>>{left, right},
+      std::vector<Underlying>{right.ptr->primal, left.ptr->primal})};
 }
 
 template <class Underlying>
@@ -224,8 +226,8 @@ operator/(const Reverse<Underlying>& left, const Reverse<Underlying>& right) {
 
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       new_primal,
-      std::list<Reverse<Underlying>>{left, right},
-      std::list<Underlying>{
+      std::vector<Reverse<Underlying>>{left, right},
+      std::vector<Underlying>{
           1 / right.ptr->primal, -new_primal / right.ptr->primal})};
 }
 
@@ -255,8 +257,8 @@ requires Number<Underlying> Reverse<Underlying> pow(
                          .derivative1;
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       new_primal,
-      std::list<Reverse<Underlying>>{base, exponent},
-      std::list<Underlying>{grad1, grad2})};
+      std::vector<Reverse<Underlying>>{base, exponent},
+      std::vector<Underlying>{grad1, grad2})};
 }
 
 template <class Underlying>
@@ -265,8 +267,8 @@ requires Number<Underlying> Reverse<Underlying> exp(
   Underlying new_primal = exp(x.ptr->primal);
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       new_primal,
-      std::list<Reverse<Underlying>>{x},
-      std::list<Underlying>{new_primal})};
+      std::vector<Reverse<Underlying>>{x},
+      std::vector<Underlying>{new_primal})};
 }
 
 template <class Underlying>
@@ -275,8 +277,8 @@ requires Number<Underlying> Reverse<Underlying> log(
   Underlying new_primal = log(x.ptr->primal);
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       new_primal,
-      std::list<Reverse<Underlying>>{x},
-      std::list<Underlying>{1 / x.ptr->primal})};
+      std::vector<Reverse<Underlying>>{x},
+      std::vector<Underlying>{1 / x.ptr->primal})};
 }
 
 template <class Underlying>
@@ -286,8 +288,8 @@ requires Number<Underlying> Reverse<Underlying> atan(
   Underlying new_derivative1 = 1 / (x.ptr->primal * x.ptr->primal + 1.0f);
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       new_primal,
-      std::list<Reverse<Underlying>>{x},
-      std::list<Underlying>{new_derivative1})};
+      std::vector<Reverse<Underlying>>{x},
+      std::vector<Underlying>{new_derivative1})};
 }
 
 template <class Underlying>
@@ -297,8 +299,8 @@ requires Number<Underlying> Reverse<Underlying> lgamma(
   Underlying new_derivative1 = polygamma(0, x.ptr->primal);
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       new_primal,
-      std::list<Reverse<Underlying>>{x},
-      std::list<Underlying>{new_derivative1})};
+      std::vector<Reverse<Underlying>>{x},
+      std::vector<Underlying>{new_derivative1})};
 }
 
 template <class Underlying>
@@ -309,8 +311,8 @@ requires Number<Underlying> Reverse<Underlying> polygamma(
   Underlying new_derivative1 = polygamma(n + 1, x.ptr->primal);
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       new_primal,
-      std::list<Reverse<Underlying>>{x},
-      std::list<Underlying>{new_derivative1})};
+      std::vector<Reverse<Underlying>>{x},
+      std::vector<Underlying>{new_derivative1})};
 }
 
 template <class Underlying>
@@ -321,8 +323,8 @@ requires Number<Underlying> Reverse<Underlying> log1p(
   Underlying new_derivative1 = 1.0 / (x_value + 1);
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       new_primal,
-      std::list<Reverse<Underlying>>{x},
-      std::list<Underlying>{new_derivative1})};
+      std::vector<Reverse<Underlying>>{x},
+      std::vector<Underlying>{new_derivative1})};
 }
 
 template <class Underlying>
@@ -340,8 +342,8 @@ requires Number<Underlying> Reverse<Underlying> if_equal(
       when_not_equal.ptr->primal);
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       new_primal,
-      std::list<Reverse<Underlying>>{when_equal, when_not_equal},
-      std::list<Underlying>{
+      std::vector<Reverse<Underlying>>{when_equal, when_not_equal},
+      std::vector<Underlying>{
           if_equal(value.ptr->primal, comparand.ptr->primal, 1, 0),
           if_equal(value.ptr->primal, comparand.ptr->primal, 0, 1)})};
 }
@@ -361,8 +363,8 @@ requires Number<Underlying> Reverse<Underlying> if_less(
       when_not_less.ptr->primal);
   return Reverse<Underlying>{std::make_shared<PrecomputedGradients<Underlying>>(
       new_primal,
-      std::list<Reverse<Underlying>>{when_less, when_not_less},
-      std::list<Underlying>{
+      std::vector<Reverse<Underlying>>{when_less, when_not_less},
+      std::vector<Underlying>{
           if_less(value.ptr->primal, comparand.ptr->primal, 1, 0),
           if_less(value.ptr->primal, comparand.ptr->primal, 0, 1)})};
 }

minibmg/dedup.h

@@ -8,7 +8,6 @@
 #pragma once
 
 #include <concepts>
-#include <list>
 #include <memory>
 #include <type_traits>
 #include <unordered_map>

minibmg/fluid_factory.h

@@ -39,7 +39,7 @@ class Graph::FluidFactory {
 
  private:
   std::vector<Nodep> queries;
-  std::list<std::pair<Nodep, double>> observations;
+  std::vector<std::pair<Nodep, double>> observations;
 };
 
 } // namespace beanmachine::minibmg

minibmg/graph.cpp

@@ -6,7 +6,6 @@
  */
 
 #include "beanmachine/minibmg/graph.h"
-#include <list>
 #include <memory>
 #include <stdexcept>
 #include <vector>
@@ -19,16 +18,16 @@ using namespace beanmachine::minibmg;
 
 const std::vector<Nodep> roots(
     const std::vector<Nodep>& queries,
-    const std::list<std::pair<Nodep, double>>& observations) {
-  std::list<Nodep> roots;
+    const std::vector<std::pair<Nodep, double>>& observations) {
+  std::vector<Nodep> roots;
   for (auto& n : queries) {
     roots.push_back(n);
   }
   for (auto& p : observations) {
     if (!std::dynamic_pointer_cast<const ScalarSampleNode>(p.first)) {
       throw std::invalid_argument(fmt::format("can only observe a sample"));
     }
-    roots.push_front(p.first);
+    roots.push_back(p.first);
   }
   std::vector<Nodep> all_nodes;
   if (!topological_sort<Nodep>(roots, &in_nodes, all_nodes)) {
@@ -40,7 +39,7 @@ const std::vector<Nodep> roots(
 
 struct QueriesAndObservations {
   std::vector<Nodep> queries;
-  std::list<std::pair<Nodep, double>> observations;
+  std::vector<std::pair<Nodep, double>> observations;
   ~QueriesAndObservations() {}
 };
 
@@ -65,7 +64,7 @@ class NodeRewriteAdapter<QueriesAndObservations> {
       const QueriesAndObservations& qo,
       const std::unordered_map<Nodep, Nodep>& map) const {
     NodeRewriteAdapter<std::vector<Nodep>> h1{};
-    NodeRewriteAdapter<std::list<std::pair<Nodep, double>>> h2{};
+    NodeRewriteAdapter<std::vector<std::pair<Nodep, double>>> h2{};
     return QueriesAndObservations{
         h1.rewrite(qo.queries, map), h2.rewrite(qo.observations, map)};
   }
@@ -75,7 +74,7 @@ using dynamic = folly::dynamic;
 
 Graph Graph::create(
     const std::vector<Nodep>& queries,
-    const std::list<std::pair<Nodep, double>>& observations,
+    const std::vector<std::pair<Nodep, double>>& observations,
     std::unordered_map<Nodep, Nodep>* built_map) {
   for (auto& p : observations) {
     if (!std::dynamic_pointer_cast<const ScalarSampleNode>(p.first)) {
@@ -95,7 +94,7 @@ Graph::~Graph() {}
 Graph::Graph(
     const std::vector<Nodep>& nodes,
     const std::vector<Nodep>& queries,
-    const std::list<std::pair<Nodep, double>>& observations)
+    const std::vector<std::pair<Nodep, double>>& observations)
     : nodes{nodes}, queries{queries}, observations{observations} {}
 
 } // namespace beanmachine::minibmg

minibmg/graph.h

@@ -8,7 +8,7 @@
 #pragma once
 
 #include <folly/json.h>
-#include <list>
+#include <vector>
 #include "beanmachine/minibmg/dedup.h"
 #include "beanmachine/minibmg/graph_properties/container.h"
 #include "beanmachine/minibmg/node.h"
@@ -27,7 +27,7 @@ class Graph : public Container {
 
   static Graph create(
       const std::vector<Nodep>& queries,
-      const std::list<std::pair<Nodep, double>>& observations,
+      const std::vector<std::pair<Nodep, double>>& observations,
       std::unordered_map<Nodep, Nodep>* built_map = nullptr);
   ~Graph();
 
@@ -55,15 +55,15 @@ class Graph : public Container {
 
   // Observations of the model.  These are SAMPLE nodes in the model whose
   // values are known.
-  const std::list<std::pair<Nodep, double>> observations;
+  const std::vector<std::pair<Nodep, double>> observations;
 
  private:
   // A private constructor that forms a graph without validation.
   // Used internally.  All exposed graphs should be validated.
   Graph(
       const std::vector<Nodep>& nodes,
       const std::vector<Nodep>& queries,
-      const std::list<std::pair<Nodep, double>>& observations);
+      const std::vector<std::pair<Nodep, double>>& observations);
 
  public:
   // A factory for making graphs, like the bmg API used by Beanstalk
@@ -99,7 +99,7 @@ class NodeRewriteAdapter<Graph> {
   Graph rewrite(const Graph& qo, const std::unordered_map<Nodep, Nodep>& map)
       const {
     NodeRewriteAdapter<std::vector<Nodep>> h1{};
-    NodeRewriteAdapter<std::list<std::pair<Nodep, double>>> h2{};
+    NodeRewriteAdapter<std::vector<std::pair<Nodep, double>>> h2{};
     return Graph::create(
         h1.rewrite(qo.queries, map), h2.rewrite(qo.observations, map));
   }

minibmg/graph_factory.cpp

@@ -10,7 +10,6 @@
 #include <stdexcept>
 #include <unordered_map>
 #include "beanmachine/minibmg/node.h"
-#include "node.h"
 
 namespace beanmachine::minibmg {
 

minibmg/graph_factory.h

@@ -8,7 +8,6 @@
 #pragma once
 
 #include <fmt/format.h>
-#include <list>
 #include <memory>
 #include <unordered_map>
 #include <vector>
@@ -123,7 +122,7 @@ class Graph::Factory {
   std::unordered_map<NodeId, Nodep> identifer_to_node;
   std::unordered_map<Nodep, NodeId> node_to_identifier;
   std::vector<Nodep> queries;
-  std::list<std::pair<Nodep, double>> observations;
+  std::vector<std::pair<Nodep, double>> observations;
   unsigned long next_identifier = 0;
 
   ScalarNodeId add_node(ScalarNodep node);

minibmg/graph_properties/observations_by_node.h

@@ -7,7 +7,6 @@
 
 #pragma once
 
-#include <list>
 #include <unordered_set>
 #include "beanmachine/minibmg/graph.h"
 #include "beanmachine/minibmg/node.h"