From 107bcd8bb4294bf4d20d3e114a1baab59eeb873c Mon Sep 17 00:00:00 2001
From: dellaert <dellaert@cc.gatech.edu>
Date: Sun, 12 Oct 2014 22:04:40 +0200
Subject: [PATCH] Going forwards, we default to reverse :-)

---
 gtsam_unstable/nonlinear/Expression.h           | 10 ++--------
 .../nonlinear/tests/testExpression.cpp          | 17 +++--------------
 .../nonlinear/tests/testExpressionFactor.cpp    |  9 +++++++++
 3 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/gtsam_unstable/nonlinear/Expression.h b/gtsam_unstable/nonlinear/Expression.h
index 147804fb1..913a2b037 100644
--- a/gtsam_unstable/nonlinear/Expression.h
+++ b/gtsam_unstable/nonlinear/Expression.h
@@ -109,10 +109,8 @@ public:
   }
 
   /// Return value and derivatives, forward AD version
-  T forward(const Values& values, JacobianMap& jacobians) const {
-    Augmented<T> augmented = root_->forward(values);
-    jacobians = augmented.jacobians();
-    return augmented.value();
+  Augmented<T> forward(const Values& values) const {
+    return root_->forward(values);
   }
 
   // Return size needed for memory buffer in traceExecution
@@ -143,11 +141,7 @@ public:
 
   /// Return value and derivatives
   T value(const Values& values, JacobianMap& jacobians) const {
-#ifdef EXPRESSION_FORWARD_AD
-    return forward(values, jacobians);
-#else
     return reverse(values, jacobians);
-#endif
   }
 
   const boost::shared_ptr<ExpressionNode<T> >& root() const {
diff --git a/gtsam_unstable/nonlinear/tests/testExpression.cpp b/gtsam_unstable/nonlinear/tests/testExpression.cpp
index 38297f156..bf13749b9 100644
--- a/gtsam_unstable/nonlinear/tests/testExpression.cpp
+++ b/gtsam_unstable/nonlinear/tests/testExpression.cpp
@@ -61,10 +61,9 @@ TEST(Expression, leaf) {
   JacobianMap expected;
   expected[100] = eye(3);
 
-  JacobianMap actualMap1;
-  Rot3 actual1 = R.forward(values, actualMap1);
-  EXPECT(assert_equal(someR, actual1));
-  EXPECT(actualMap1 == expected);
+  Augmented<Rot3> actual1 = R.forward(values);
+  EXPECT(assert_equal(someR, actual1.value()));
+  EXPECT(actual1.jacobians() == expected);
 
   JacobianMap actualMap2;
   Rot3 actual2 = R.reverse(values, actualMap2);
@@ -126,16 +125,6 @@ TEST(Expression, keys_tree) {
   EXPECT(expectedKeys == tree::uv_hat.keys());
 }
 /* ************************************************************************* */
-// keys
-TEST(Expression, block_tree) {
-//  // Check VerticalBlockMatrix
-//  size_t dimensions[3] = { 6, 3, 5 };
-//  Matrix matrix(2, 14);
-//  VerticalBlockMatrix expected(dimensions, matrix), actual =
-//      tree::uv_hat.verticalBlockMatrix();
-//  EXPECT( assert_equal(expected, *jf, 1e-9));
-}
-/* ************************************************************************* */
 
 TEST(Expression, compose1) {
 
diff --git a/gtsam_unstable/nonlinear/tests/testExpressionFactor.cpp b/gtsam_unstable/nonlinear/tests/testExpressionFactor.cpp
index eb4f1e52e..015a4ca6e 100644
--- a/gtsam_unstable/nonlinear/tests/testExpressionFactor.cpp
+++ b/gtsam_unstable/nonlinear/tests/testExpressionFactor.cpp
@@ -260,6 +260,15 @@ TEST(ExpressionFactor, tree) {
   Point2_ xy_hat(PinholeCamera<Cal3_S2>::project_to_camera, p_cam);
   Point2_ uv_hat(K, &Cal3_S2::uncalibrate, xy_hat);
 
+  // Compare reverse and forward
+  {
+  JacobianMap expectedMap; // via reverse
+  Point2 expectedValue = uv_hat.reverse(values, expectedMap);
+  Augmented<Point2> actual = uv_hat.forward(values);
+  EXPECT(assert_equal(expectedValue, actual.value()));
+  EXPECT(actual.jacobians() == expectedMap);
+  }
+
   // Create factor and check value, dimension, linearization
   ExpressionFactor<Point2> f(model, measured, uv_hat);
   EXPECT_DOUBLES_EQUAL(expected_error, f.error(values), 1e-9);