From 4fa192248de37473fbc4a7e8ba8c9549bfa7c590 Mon Sep 17 00:00:00 2001
From: Frank Dellaert <dellaert@gmail.com>
Date: Sun, 29 Sep 2024 00:21:40 -0700
Subject: [PATCH] Address o1 review comments

---
 gtsam/hybrid/HybridGaussianConditional.cpp | 17 ++++++++++-------
 gtsam/hybrid/HybridGaussianConditional.h   |  6 +++---
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/gtsam/hybrid/HybridGaussianConditional.cpp b/gtsam/hybrid/HybridGaussianConditional.cpp
index c348b85f0a..b7c79a5b27 100644
--- a/gtsam/hybrid/HybridGaussianConditional.cpp
+++ b/gtsam/hybrid/HybridGaussianConditional.cpp
@@ -43,18 +43,20 @@ struct HybridGaussianConditional::Helper {
 
   /// Construct from a vector of mean and sigma pairs, plus extra args.
   template <typename... Args>
-  Helper(const DiscreteKey &mode, const P &p, Args &&...args) {
+  explicit Helper(const DiscreteKey &mode, const P &p, Args &&...args) {
     nrFrontals = 1;
     minNegLogConstant = std::numeric_limits<double>::infinity();
 
     std::vector<GaussianFactorValuePair> fvs;
     std::vector<GC::shared_ptr> gcs;
+    fvs.reserve(p.size());
+    gcs.reserve(p.size());
     for (const auto &[mean, sigma] : p) {
       auto gaussianConditional =
           GC::sharedMeanAndStddev(std::forward<Args>(args)..., mean, sigma);
       double value = gaussianConditional->negLogConstant();
       minNegLogConstant = std::min(minNegLogConstant, value);
-      fvs.push_back({gaussianConditional, value});
+      fvs.emplace_back(gaussianConditional, value);
       gcs.push_back(gaussianConditional);
     }
 
@@ -63,7 +65,7 @@ struct HybridGaussianConditional::Helper {
   }
 
   /// Construct from tree of GaussianConditionals.
-  Helper(const Conditionals &conditionals)
+  explicit Helper(const Conditionals &conditionals)
       : conditionals(conditionals),
         minNegLogConstant(std::numeric_limits<double>::infinity()) {
     auto func = [this](const GC::shared_ptr &c) -> GaussianFactorValuePair {
@@ -80,7 +82,8 @@ struct HybridGaussianConditional::Helper {
     pairs = FactorValuePairs(conditionals, func);
     if (!nrFrontals.has_value()) {
       throw std::runtime_error(
-          "HybridGaussianConditional: need at least one frontal variable.");
+          "HybridGaussianConditional: need at least one frontal variable. "
+          "Provided conditionals do not contain any frontal variables.");
     }
   }
 };
@@ -100,20 +103,20 @@ HybridGaussianConditional::HybridGaussianConditional(
                                 Conditionals({mode}, conditionals)) {}
 
 HybridGaussianConditional::HybridGaussianConditional(
-    const DiscreteKey mode, Key key,  //
+    const DiscreteKey &mode, Key key,  //
     const std::vector<std::pair<Vector, double>> &parameters)
     : HybridGaussianConditional(DiscreteKeys{mode},
                                 Helper(mode, parameters, key)) {}
 
 HybridGaussianConditional::HybridGaussianConditional(
-    const DiscreteKey mode, Key key,  //
+    const DiscreteKey &mode, Key key,  //
     const Matrix &A, Key parent,
     const std::vector<std::pair<Vector, double>> &parameters)
     : HybridGaussianConditional(DiscreteKeys{mode},
                                 Helper(mode, parameters, key, A, parent)) {}
 
 HybridGaussianConditional::HybridGaussianConditional(
-    const DiscreteKey mode, Key key,  //
+    const DiscreteKey &mode, Key key,  //
     const Matrix &A1, Key parent1, const Matrix &A2, Key parent2,
     const std::vector<std::pair<Vector, double>> &parameters)
     : HybridGaussianConditional(
diff --git a/gtsam/hybrid/HybridGaussianConditional.h b/gtsam/hybrid/HybridGaussianConditional.h
index 7c14f00085..16fc0500f6 100644
--- a/gtsam/hybrid/HybridGaussianConditional.h
+++ b/gtsam/hybrid/HybridGaussianConditional.h
@@ -89,18 +89,18 @@ class GTSAM_EXPORT HybridGaussianConditional
 
   /// Construct from mean `mu_i` and `sigma_i`.
   HybridGaussianConditional(
-      const DiscreteKey mode, Key key,  //
+      const DiscreteKey &mode, Key key,  //
       const std::vector<std::pair<Vector, double>> &parameters);
 
   /// Construct from conditional mean `A1 p1 + b_i` and `sigma_i`.
   HybridGaussianConditional(
-      const DiscreteKey mode, Key key,  //
+      const DiscreteKey &mode, Key key,  //
       const Matrix &A, Key parent,
       const std::vector<std::pair<Vector, double>> &parameters);
 
   /// Construct from conditional mean `A1 p1 + A2 p2 + b_i` and `sigma_i`.
   HybridGaussianConditional(
-      const DiscreteKey mode, Key key,  //
+      const DiscreteKey &mode, Key key,  //
       const Matrix &A1, Key parent1, const Matrix &A2, Key parent2,
       const std::vector<std::pair<Vector, double>> &parameters);