Cleanups

gtsam/inference/VariableSlots.h

@@ -32,25 +32,23 @@
 
 namespace gtsam {
 
-/**
+/** A combined factor is assembled as one block of rows for each component
- * A combined factor is assembled as one block of rows for each component
+*   factor.  In each row-block (factor), some of the column-blocks (variables)
- * factor.  In each row-block (factor), some of the column-blocks (variables)
+*   may be empty since factors involving different sets of variables are
- * may be empty since factors involving different sets of variables are
+*   interleaved.
- * interleaved.
+*  
- *
+*   VariableSlots describes the 2D block structure of the combined factor.  It
- * VariableSlots describes the 2D block structure of the combined factor.  It
+*   is a map<Index, vector<size_t> >.  The Index is the real
- * is essentially a map<Index, vector<size_t> >.  The Index is the real
+*   variable index of the combined factor slot.  The vector<size_t> tells, for
- * variable index of the combined factor slot.  The vector<size_t> tells, for
+*   each row-block (factor), which column-block (variable slot) from the
- * each row-block (factor), which column-block (variable slot) from the
+*   component factor appears in this block of the combined factor.
- * component factor appears in this block of the combined factor.
+*  
- *
+*   As an example, if the combined factor contains variables 1, 3, and 5, then
- * As an example, if the combined factor contains variables 1, 3, and 5, then
+*   "variableSlots[3][2] == 0" indicates that column-block 1 (corresponding to
- * "variableSlots[3][2] == 0" indicates that column-block 1 (corresponding to
+*   variable index 3), row-block 2 (also meaning component factor 2), comes from
- * variable index 3), row-block 2 (also meaning component factor 2), comes from
+*   column-block 0 of component factor 2.
- * column-block 0 of component factor 2.
+*  
- *
+*   \nosubgrouping */
- * \nosubgrouping
- */
 
 class VariableSlots : public FastMap<Index, std::vector<size_t> > {
 

gtsam/linear/JacobianFactor-inl.h

@@ -37,7 +37,7 @@ namespace gtsam {
   template<typename KEYS>
   JacobianFactor::JacobianFactor(
     const KEYS& keys, const VerticalBlockMatrix& augmentedMatrix, const SharedDiagonal& model) :
-  Base(keys)
+  Base(keys), Ab_(augmentedMatrix)
   {
     // Check noise model dimension
     if(model && model->dim() != augmentedMatrix.rows())
@@ -55,10 +55,6 @@ namespace gtsam {
       "Error in JacobianFactor constructor input.  The last provided matrix block\n"
       "must be the RHS vector, but the last provided block had more than one column.");
 
-    // Allocate and copy matrix - only takes the active view of the provided augmented matrix
-    Ab_ = VerticalBlockMatrix::LikeActiveViewOf(augmentedMatrix);
-    Ab_.full() = augmentedMatrix.full();
-
     // Take noise model
     model_ = model;
   }

gtsam/linear/JacobianFactor.cpp

@@ -210,13 +210,11 @@ namespace gtsam {
     gttic(JacobianFactor_combine_constructor);
 
     // Compute VariableSlots if one was not provided
-    gttic(Compute_VariableSlots);
     boost::optional<VariableSlots> computedVariableSlots;
     if(!variableSlots) {
       computedVariableSlots = VariableSlots(graph);
       variableSlots = computedVariableSlots; // Binds reference, does not copy VariableSlots
     }
-    gttoc(Compute_VariableSlots);
 
     // Cast or convert to Jacobians
     std::vector<JacobianFactor::shared_ptr> jacobians = _convertOrCastToJacobians(graph);
@@ -516,8 +514,7 @@ namespace gtsam {
     JacobianFactor::shared_ptr jointFactor;
     try {
       jointFactor = boost::make_shared<JacobianFactor>(factors, keys);
-    } catch(std::invalid_argument& e) {
+    } catch(std::invalid_argument&) {
-      (void) e; // Avoid unused variable warning
       throw InvalidDenseElimination(
         "EliminateQR was called with a request to eliminate variables that are not\n"
         "involved in the provided factors.");
@@ -542,6 +539,8 @@ namespace gtsam {
   /* ************************************************************************* */
   GaussianConditional::shared_ptr JacobianFactor::splitConditional(size_t nrFrontals)
   {
+    gttic(JacobianFactor_splitConditional);
+
     if(nrFrontals > size())
       throw std::invalid_argument("Requesting to split more variables than exist using JacobianFactor::splitConditional");
 

gtsam/linear/tests/testGaussianBayesNetUnordered.cpp

@@ -40,7 +40,7 @@ static GaussianBayesNet smallBayesNet = list_of
   (GaussianConditional(_y_, Vector_(1, 5.0), Matrix_(1, 1, 1.0)));
 
 /* ************************************************************************* */
-TEST( GaussianBayesNetOrdered, matrix )
+TEST( GaussianBayesNet, matrix )
 {
   Matrix R; Vector d;
   boost::tie(R,d) = smallBayesNet.matrix(); // find matrix and RHS
@@ -56,7 +56,7 @@ TEST( GaussianBayesNetOrdered, matrix )
 }
 
 /* ************************************************************************* */
-TEST( GaussianBayesNetOrdered, optimize )
+TEST( GaussianBayesNet, optimize )
 {
   VectorValues actual = smallBayesNet.optimize();
 
@@ -68,7 +68,7 @@ TEST( GaussianBayesNetOrdered, optimize )
 }
 
 /* ************************************************************************* */
-TEST( GaussianBayesNetOrdered, optimize3 )
+TEST( GaussianBayesNet, optimize3 )
 {
   // y = R*x, x=inv(R)*y
   // 4 = 1 1   -1 
@@ -88,7 +88,7 @@ TEST( GaussianBayesNetOrdered, optimize3 )
 }
 
 /* ************************************************************************* */
-TEST( GaussianBayesNetOrdered, backSubstituteTranspose )
+TEST( GaussianBayesNet, backSubstituteTranspose )
 {
   // x=R'*y, expected=inv(R')*x
   // 2 = 1    2
@@ -107,7 +107,7 @@ TEST( GaussianBayesNetOrdered, backSubstituteTranspose )
 
 /* ************************************************************************* */
 // Tests computing Determinant
-TEST( GaussianBayesNetOrdered, DeterminantTest )
+TEST( GaussianBayesNet, DeterminantTest )
 {
   GaussianBayesNet cbn;
   cbn += GaussianConditional(

gtsam/linear/tests/testJacobianFactorUnordered.cpp

@@ -228,9 +228,9 @@ TEST(JacobianFactor, matrices)
   EXPECT(assert_equal(simple::noise->R() * augmentedJacobianExpected, factor.augmentedJacobian()));
 
   // Unwhitened Jacobian
-  EXPECT(assert_equal(jacobianExpected, factor.jacobian(false).first));
+  EXPECT(assert_equal(jacobianExpected, factor.jacobianUnweighted().first));
-  EXPECT(assert_equal(rhsExpected, factor.jacobian(false).second));
+  EXPECT(assert_equal(rhsExpected, factor.jacobianUnweighted().second));
-  EXPECT(assert_equal(augmentedJacobianExpected, factor.augmentedJacobian(false)));
+  EXPECT(assert_equal(augmentedJacobianExpected, factor.augmentedJacobianUnweighted()));
 }
 
 /* ************************************************************************* */