From 51b6fd0b43852bf14148b39c14d2563f2d87c3d4 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Tue, 3 Nov 2020 12:11:17 -0500 Subject: [PATCH 1/5] Added flag for absolute error --- gtsam/base/Matrix.h | 2 +- gtsam/base/Vector.cpp | 6 ++++-- gtsam/base/Vector.h | 3 ++- gtsam/geometry/tests/testRot3.cpp | 6 +++--- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/gtsam/base/Matrix.h b/gtsam/base/Matrix.h index 071c33fca..9adf4e1c1 100644 --- a/gtsam/base/Matrix.h +++ b/gtsam/base/Matrix.h @@ -90,7 +90,7 @@ bool equal_with_abs_tol(const Eigen::DenseBase& A, const Eigen::DenseBas for(size_t i=0; i::max())) { + else if (abs(a - b) <= + tol * min(larger, std::numeric_limits::max()) && + !absolute) { return true; } diff --git a/gtsam/base/Vector.h b/gtsam/base/Vector.h index 319ad6ee6..b0fc74f26 100644 --- a/gtsam/base/Vector.h +++ b/gtsam/base/Vector.h @@ -87,7 +87,8 @@ static_assert( * * Return true if two numbers are close wrt tol. */ -GTSAM_EXPORT bool fpEqual(double a, double b, double tol); +GTSAM_EXPORT bool fpEqual(double a, double b, double tol, + bool absolute = false); /** * print without optional string, must specify cout yourself diff --git a/gtsam/geometry/tests/testRot3.cpp b/gtsam/geometry/tests/testRot3.cpp index 7b792f8bd..889f68580 100644 --- a/gtsam/geometry/tests/testRot3.cpp +++ b/gtsam/geometry/tests/testRot3.cpp @@ -807,15 +807,15 @@ TEST(Rot3, RQ_derivative) { test_xyz.push_back(VecAndErr{{0, 0, 0}, error}); test_xyz.push_back(VecAndErr{{0, 0.5, -0.5}, error}); test_xyz.push_back(VecAndErr{{0.3, 0, 0.2}, error}); - test_xyz.push_back(VecAndErr{{-0.6, 1.3, 0}, error}); + test_xyz.push_back(VecAndErr{{-0.6, 1.3, 0}, 1e-8}); test_xyz.push_back(VecAndErr{{1.0, 0.7, 0.8}, error}); test_xyz.push_back(VecAndErr{{3.0, 0.7, -0.6}, error}); test_xyz.push_back(VecAndErr{{M_PI / 2, 0, 0}, error}); test_xyz.push_back(VecAndErr{{0, 0, M_PI / 2}, error}); // Test close to singularity - test_xyz.push_back(VecAndErr{{0, M_PI / 2 - 1e-1, 0}, 1e-8}); - test_xyz.push_back(VecAndErr{{0, 3 * M_PI / 2 + 1e-1, 0}, 1e-8}); + test_xyz.push_back(VecAndErr{{0, M_PI / 2 - 1e-1, 0}, 1e-7}); + test_xyz.push_back(VecAndErr{{0, 3 * M_PI / 2 + 1e-1, 0}, 1e-7}); test_xyz.push_back(VecAndErr{{0, M_PI / 2 - 1.1e-2, 0}, 1e-4}); test_xyz.push_back(VecAndErr{{0, 3 * M_PI / 2 + 1.1e-2, 0}, 1e-4}); From c68a64745e1ad99cb047286434cfda1e1b7aedc7 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Tue, 3 Nov 2020 14:27:05 -0500 Subject: [PATCH 2/5] add test --- gtsam/base/tests/testMatrix.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/gtsam/base/tests/testMatrix.cpp b/gtsam/base/tests/testMatrix.cpp index 468e842f2..15087093a 100644 --- a/gtsam/base/tests/testMatrix.cpp +++ b/gtsam/base/tests/testMatrix.cpp @@ -1163,6 +1163,19 @@ TEST(Matrix , IsVectorSpace) { BOOST_CONCEPT_ASSERT((IsVectorSpace)); } +TEST(Matrix, AbsoluteError) { + double a = 2000, b = 1997, tol=1e-1; + bool isEqual; + + // Test absolute error + isEqual = fpEqual(a, b, tol, true); + EXPECT(isEqual == false); + + // Test relative error + isEqual = fpEqual(a, b, tol, false); + EXPECT(isEqual == true); +} + /* ************************************************************************* */ int main() { TestResult tr; From 326957b0d35ece7ae44cf5d06be42798c9f577dd Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Tue, 3 Nov 2020 19:31:39 -0500 Subject: [PATCH 3/5] cleaner assertion --- gtsam/base/tests/testMatrix.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gtsam/base/tests/testMatrix.cpp b/gtsam/base/tests/testMatrix.cpp index 15087093a..d22ffc0db 100644 --- a/gtsam/base/tests/testMatrix.cpp +++ b/gtsam/base/tests/testMatrix.cpp @@ -1169,11 +1169,11 @@ TEST(Matrix, AbsoluteError) { // Test absolute error isEqual = fpEqual(a, b, tol, true); - EXPECT(isEqual == false); + EXPECT(!isEqual); // Test relative error isEqual = fpEqual(a, b, tol, false); - EXPECT(isEqual == true); + EXPECT(isEqual); } /* ************************************************************************* */ From 1fd0e57fb0a2a9cd24accd3d8dc04d91328dccf6 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 18 Nov 2020 14:48:05 -0500 Subject: [PATCH 4/5] 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& A, const Eigen::DenseBas for(size_t i=0; i 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::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); } From 0737a4594a1c2a5eb814b6da060ffcab8749d1f6 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Thu, 19 Nov 2020 07:44:42 -0500 Subject: [PATCH 5/5] better flag name and docs --- gtsam/base/Vector.cpp | 4 ++-- gtsam/base/Vector.h | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/gtsam/base/Vector.cpp b/gtsam/base/Vector.cpp index d2f3ae868..658ab9a0d 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 check_relative) { +bool fpEqual(double a, double b, double tol, bool check_relative_also) { using std::abs; using std::isnan; using std::isinf; @@ -68,7 +68,7 @@ bool fpEqual(double a, double b, double tol, bool check_relative) { // Check for relative error else if (abs(a - b) <= tol * min(larger, std::numeric_limits::max()) && - check_relative) { + check_relative_also) { return true; } diff --git a/gtsam/base/Vector.h b/gtsam/base/Vector.h index 49b7e6d9d..ed90a7126 100644 --- a/gtsam/base/Vector.h +++ b/gtsam/base/Vector.h @@ -85,13 +85,15 @@ 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. + * @param check_relative_also is a flag which toggles additional checking for + * relative error. This means that if either the absolute error or the relative + * error is within the tolerance, the result will be true. + * 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 check_relative = true); + bool check_relative_also = true); /** * print without optional string, must specify cout yourself