From 1fd0e57fb0a2a9cd24accd3d8dc04d91328dccf6 Mon Sep 17 00:00:00 2001
From: Varun Agrawal <varagrawal@gmail.com>
Date: Wed, 18 Nov 2020 14:48:05 -0500
Subject: [PATCH] Better fkag naming, and more docs

---
 gtsam/base/Matrix.h             |  2 +-
 gtsam/base/Vector.cpp           | 14 +++++++-------
 gtsam/base/Vector.h             |  5 ++++-
 gtsam/base/tests/testMatrix.cpp | 10 +++++-----
 4 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/gtsam/base/Matrix.h b/gtsam/base/Matrix.h
index 9adf4e1c1..a3a14c6c3 100644
--- a/gtsam/base/Matrix.h
+++ b/gtsam/base/Matrix.h
@@ -90,7 +90,7 @@ bool equal_with_abs_tol(const Eigen::DenseBase<MATRIX>& A, const Eigen::DenseBas
 
   for(size_t i=0; i<m1; i++)
     for(size_t j=0; j<n1; j++) {
-      if(!fpEqual(A(i,j), B(i,j), tol, true)) {
+      if(!fpEqual(A(i,j), B(i,j), tol, false)) {
         return false;
       }
     }
diff --git a/gtsam/base/Vector.cpp b/gtsam/base/Vector.cpp
index ae4d75455..d2f3ae868 100644
--- a/gtsam/base/Vector.cpp
+++ b/gtsam/base/Vector.cpp
@@ -39,7 +39,7 @@ namespace gtsam {
  * 1. https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/
  * 2. https://floating-point-gui.de/errors/comparison/
  * ************************************************************************* */
-bool fpEqual(double a, double b, double tol, bool absolute) {
+bool fpEqual(double a, double b, double tol, bool check_relative) {
   using std::abs;
   using std::isnan;
   using std::isinf;
@@ -48,7 +48,7 @@ bool fpEqual(double a, double b, double tol, bool absolute) {
   double larger = (abs(b) > abs(a)) ? abs(b) : abs(a);
 
   // handle NaNs
-  if(std::isnan(a) || isnan(b)) {
+  if(isnan(a) || isnan(b)) {
     return isnan(a) && isnan(b);
   }
   // handle inf
@@ -60,15 +60,15 @@ bool fpEqual(double a, double b, double tol, bool absolute) {
   else if(a == 0 || b == 0 || (abs(a) + abs(b)) < DOUBLE_MIN_NORMAL) {
     return abs(a-b) <= tol * DOUBLE_MIN_NORMAL;
   }
-  // Check if the numbers are really close
-  // Needed when comparing numbers near zero or tol is in vicinity
-  else if(abs(a-b) <= tol) {
+  // Check if the numbers are really close.
+  // Needed when comparing numbers near zero or tol is in vicinity.
+  else if (abs(a - b) <= tol) {
     return true;
   }
-  // Use relative error
+  // Check for relative error
   else if (abs(a - b) <=
                tol * min(larger, std::numeric_limits<double>::max()) &&
-           !absolute) {
+           check_relative) {
     return true;
   }
 
diff --git a/gtsam/base/Vector.h b/gtsam/base/Vector.h
index b0fc74f26..49b7e6d9d 100644
--- a/gtsam/base/Vector.h
+++ b/gtsam/base/Vector.h
@@ -85,10 +85,13 @@ static_assert(
  * respectively for the comparison to be true.
  * If one is NaN/Inf and the other is not, returns false.
  *
+ * The `check_relative` flag toggles checking for relative error as well. By
+ * default, the flag is true.
+ *
  * Return true if two numbers are close wrt tol.
  */
 GTSAM_EXPORT bool fpEqual(double a, double b, double tol,
-                          bool absolute = false);
+                          bool check_relative = true);
 
 /**
  * print without optional string, must specify cout yourself
diff --git a/gtsam/base/tests/testMatrix.cpp b/gtsam/base/tests/testMatrix.cpp
index d22ffc0db..a7c218705 100644
--- a/gtsam/base/tests/testMatrix.cpp
+++ b/gtsam/base/tests/testMatrix.cpp
@@ -1164,15 +1164,15 @@ TEST(Matrix , IsVectorSpace) {
 }
 
 TEST(Matrix, AbsoluteError) {
-  double a = 2000, b = 1997, tol=1e-1;
+  double a = 2000, b = 1997, tol = 1e-1;
   bool isEqual;
 
-  // Test absolute error
-  isEqual = fpEqual(a, b, tol, true);
+  // Test only absolute error
+  isEqual = fpEqual(a, b, tol, false);
   EXPECT(!isEqual);
 
-  // Test relative error
-  isEqual = fpEqual(a, b, tol, false);
+  // Test relative error as well
+  isEqual = fpEqual(a, b, tol);
   EXPECT(isEqual);
 }