From c0262b074c04e8e3ca75e66c01b08a0dc79bb641 Mon Sep 17 00:00:00 2001
From: Frank Dellaert <dellaert@gmail.com>
Date: Sun, 29 Aug 2021 19:38:20 -0400
Subject: [PATCH] Address review comments, docs only.

---
 gtsam/slam/ProjectionFactor.h | 3 ++-
 gtsam/slam/ReadMe.md          | 8 +++++++-
 gtsam/slam/SmartFactorBase.h  | 2 +-
 gtsam_unstable/slam/ReadMe.md | 1 +
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/gtsam/slam/ProjectionFactor.h b/gtsam/slam/ProjectionFactor.h
index 169fe8a0a..42dba8bd0 100644
--- a/gtsam/slam/ProjectionFactor.h
+++ b/gtsam/slam/ProjectionFactor.h
@@ -35,7 +35,8 @@ namespace gtsam {
    * The main building block for visual SLAM.
    * @addtogroup SLAM
    */
-  template<class POSE=Pose3, class LANDMARK=Point3, class CALIBRATION = Cal3_S2>
+  template <class POSE = Pose3, class LANDMARK = Point3,
+            class CALIBRATION = Cal3_S2>
   class GenericProjectionFactor: public NoiseModelFactor2<POSE, LANDMARK> {
   protected:
 
diff --git a/gtsam/slam/ReadMe.md b/gtsam/slam/ReadMe.md
index 22a5778f2..c43f0769a 100644
--- a/gtsam/slam/ReadMe.md
+++ b/gtsam/slam/ReadMe.md
@@ -2,7 +2,7 @@
 
 ## GenericProjectionFactor (defined in ProjectionFactor.h)
 
-Non-linear factor for a constraint derived from a 2D measurement. 
+Non-linear factor that minimizes the re-projection error with respect to a 2D measurement. 
 The calibration is assumed known and passed in the constructor.
 The main building block for visual SLAM.
 
@@ -14,6 +14,7 @@ Templated on
 ## SmartFactors
 
 These are "structure-less" factors, i.e., rather than introducing a new variable for an observed 3D point or landmark, a single factor is created that provides a multi-view constraint on several poses and/or cameras.
+While one typically adds multiple GenericProjectionFactors (one for each observation of a landmark), a SmartFactor collects all measurements for a landmark, i.e., the factor graph contains 1 smart factor per landmark.
 
 ### SmartFactorBase
 
@@ -46,6 +47,11 @@ Same as `SmartProjectionPoseFactor`, except:
 
 TODO: DimPose and ZDim are hardcoded. Copy/paste from `SmartProjectionPoseFactor`. Unclear what the use case is.
 
+## Linearized Smart Factors
+
+The factors below are less likely to be relevant to the user, but result from using the non-linear smart factors above.
+
+
 ### RegularImplicitSchurFactor
 
 A specialization of a GaussianFactor to structure-less SFM, which is very fast in a conjugate gradient (CG) solver. 
diff --git a/gtsam/slam/SmartFactorBase.h b/gtsam/slam/SmartFactorBase.h
index f815200ce..ddf56b289 100644
--- a/gtsam/slam/SmartFactorBase.h
+++ b/gtsam/slam/SmartFactorBase.h
@@ -44,7 +44,7 @@ namespace gtsam {
  * factors. The methods take a CameraSet<CAMERA> argument and the value of a
  * point, which is kept in the derived class.
  *
- * @tparam CAMERA should behave like a set of PinholeCamera.
+ * @tparam CAMERA should behave like a PinholeCamera.
  */
 template<class CAMERA>
 class SmartFactorBase: public NonlinearFactor {
diff --git a/gtsam_unstable/slam/ReadMe.md b/gtsam_unstable/slam/ReadMe.md
index 78d53090a..9aa0fed78 100644
--- a/gtsam_unstable/slam/ReadMe.md
+++ b/gtsam_unstable/slam/ReadMe.md
@@ -24,6 +24,7 @@ TODO: Again, as no template arguments, we could move a lot to .cpp file.
 ### SmartStereoProjectionFactorPP
 
 Similar `SmartStereoProjectionPoseFactor` but *additionally* adds an array of body_P_cam poses. The dimensions seem to be hardcoded and the types defined in the SmartFactorBase have been re-defined.  
+The body_P_cam poses are optimized here!
 
 TODO: See above, same issues as `SmartStereoProjectionPoseFactor`.