Skip to content

Commit b2560d3

Browse files
committed
handle NaNs according to PEP 485
1 parent 802eedb commit b2560d3

File tree

4 files changed

+97
-62
lines changed

4 files changed

+97
-62
lines changed

Framework/Kernel/inc/MantidKernel/FloatingPointComparison.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,18 @@
1414
namespace Mantid {
1515
namespace Kernel {
1616
/// Test for equality of doubles using compiler-defined precision
17-
template <typename T> MANTID_KERNEL_DLL bool equals(const T x, const T y);
17+
template <typename T> MANTID_KERNEL_DLL bool equals(T const x, T const y);
1818
/// Test whether x<=y within machine precision
19-
template <typename T> MANTID_KERNEL_DLL bool ltEquals(const T x, const T y);
19+
template <typename T> MANTID_KERNEL_DLL bool ltEquals(T const x, T const y);
2020
/// Test whether x>=y within machine precision
21-
template <typename T> MANTID_KERNEL_DLL bool gtEquals(const T x, const T y);
21+
template <typename T> MANTID_KERNEL_DLL bool gtEquals(T const x, T const y);
2222
/// Calculate absolute difference between x, y
23-
template <typename T> MANTID_KERNEL_DLL T absoluteDifference(const T x, const T y);
23+
template <typename T> MANTID_KERNEL_DLL T absoluteDifference(T const x, T const y);
2424
/// Calculate relative difference between x, y
25-
template <typename T> MANTID_KERNEL_DLL T relativeDifference(const T x, const T y);
25+
template <typename T> MANTID_KERNEL_DLL T relativeDifference(T const x, T const y);
2626
/// Test whether x, y are within absolute tolerance tol
27-
template <typename T> MANTID_KERNEL_DLL bool withinAbsoluteDifference(const T x, const T y, const T tolerance);
27+
template <typename T> MANTID_KERNEL_DLL bool withinAbsoluteDifference(T const x, T const y, T const tolerance);
2828
/// Test whether x, y are within relative tolerance tol
29-
template <typename T> MANTID_KERNEL_DLL bool withinRelativeDifference(const T x, const T y, const T tolerance);
29+
template <typename T> MANTID_KERNEL_DLL bool withinRelativeDifference(T const x, T const y, T const tolerance);
3030
} // namespace Kernel
3131
} // namespace Mantid

Framework/Kernel/src/FloatingPointComparison.cpp

Lines changed: 35 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,9 @@ namespace Mantid::Kernel {
2121
* @param x :: LHS comparator
2222
* @param y :: RHS comparator
2323
* @returns True if the numbers are considered equal within the given tolerance,
24-
* false otherwise. False if either is NaN
24+
* false otherwise. False if any value is NaN.
2525
*/
26-
template <typename TYPE> bool equals(const TYPE x, const TYPE y) {
27-
// bool const xnan(std::isnan(x)), ynan(std::isnan(y));
28-
// if (xnan || ynan){
29-
// if (xnan && ynan)
30-
// return true;
31-
// else
32-
// return false;
33-
// }
34-
// else {
35-
return !(std::abs(x - y) > std::numeric_limits<TYPE>::epsilon());
36-
// }
37-
}
26+
template <typename T> bool equals(T const x, T const y) { return std::abs(x - y) <= std::numeric_limits<T>::epsilon(); }
3827

3928
/**
4029
* Compare two floating-point numbers as to whether they satisfy x<=y within
@@ -44,7 +33,7 @@ template <typename TYPE> bool equals(const TYPE x, const TYPE y) {
4433
* @returns True if the numbers are considered <= within the machine tolerance,
4534
* false otherwise
4635
*/
47-
template <typename T> MANTID_KERNEL_DLL bool ltEquals(const T x, const T y) { return (equals(x, y) || x < y); }
36+
template <typename T> MANTID_KERNEL_DLL bool ltEquals(T const x, T const y) { return (equals(x, y) || x < y); }
4837

4938
/**
5039
* Compare two floating-point numbers as to whether they satisfy x>=y within
@@ -54,7 +43,7 @@ template <typename T> MANTID_KERNEL_DLL bool ltEquals(const T x, const T y) { re
5443
* @returns True if the numbers are considered <= within the machine tolerance,
5544
* false otherwise
5645
*/
57-
template <typename T> MANTID_KERNEL_DLL bool gtEquals(const T x, const T y) { return (equals(x, y) || x > y); }
46+
template <typename T> MANTID_KERNEL_DLL bool gtEquals(T const x, T const y) { return (equals(x, y) || x > y); }
5847

5948
/// ABSOLUTE AND RELATIVE DIFFERENCE
6049

@@ -64,19 +53,20 @@ template <typename T> MANTID_KERNEL_DLL bool gtEquals(const T x, const T y) { re
6453
* @param y :: second value
6554
* @returns the value of the absolute difference
6655
*/
67-
template <typename T> T absoluteDifference(const T x, const T y) { return std::abs(x - y); }
56+
template <typename T> T absoluteDifference(T const x, T const y) { return std::abs(x - y); }
6857

6958
/**
7059
* Calculate the relative difference of two floating-point numbers
7160
* @param x :: first value
7261
* @param y :: second value
73-
* @returns True if the numbers are considered equal within the given tolerance,
74-
* false otherwise
62+
* @returns the value of the relative difference. Do NOT use this
63+
* to compare the result to a tolerance; for that, use withinRelativeDifference
64+
* instead, as it will be more efficient at the comparison.
7565
*/
76-
template <typename T> T relativeDifference(const T x, const T y) {
66+
template <typename T> T relativeDifference(T const x, T const y) {
7767
// calculate numerator |x-y|
7868
T const num = absoluteDifference<T>(x, y);
79-
if (!(num > std::numeric_limits<T>::epsilon())) {
69+
if (num <= std::numeric_limits<T>::epsilon()) {
8070
// if |x-y| == 0.0 (within machine tolerance), relative difference is zero
8171
return 0.0;
8272
} else {
@@ -94,9 +84,9 @@ template <typename T> T relativeDifference(const T x, const T y) {
9484
* @param y :: second value
9585
* @param tolerance :: the tolerance
9686
* @returns True if the numbers are considered equal within the given tolerance,
97-
* false otherwise
87+
* false otherwise. False if either value is NaN.
9888
*/
99-
template <typename T> bool withinAbsoluteDifference(const T x, const T y, const T tolerance) {
89+
template <typename T> bool withinAbsoluteDifference(T const x, T const y, T const tolerance) {
10090
return ltEquals(absoluteDifference<T>(x, y), tolerance);
10191
}
10292

@@ -107,17 +97,22 @@ template <typename T> bool withinAbsoluteDifference(const T x, const T y, const
10797
* @param y :: second value
10898
* @param tolerance :: the tolerance
10999
* @returns True if the numbers are considered equal within the given tolerance,
110-
* false otherwise
100+
* false otherwise. False if either value is NaN.
111101
*/
112-
template <typename T> bool withinRelativeDifference(const T x, const T y, const T tolerance) {
102+
template <typename T> bool withinRelativeDifference(T const x, T const y, T const tolerance) {
103+
// handle the case of NaNs
104+
if (std::isnan(x) || std::isnan(y)) {
105+
return false;
106+
}
113107
T const num = absoluteDifference<T>(x, y);
114108
if (!(num > std::numeric_limits<T>::epsilon())) {
115109
// if |x-y| == 0.0 (within machine tolerance), this test passes
116110
return true;
117111
} else {
118112
// otherwise we have to calculate the denominator
119-
T const denom = static_cast<T>((std::abs(x) + std::abs(y)) / 2.);
120-
// if denom <= 1, it will only make the numerator larger
113+
T const denom = static_cast<T>(0.5 * (std::abs(x) + std::abs(y)));
114+
// if denom <= 1, then |x-y| > tol implies |x-y|/denom > tol, can return early
115+
// NOTE can only return early if BOTH denom > tol AND |x-y| > tol.
121116
if (denom <= 1. && !ltEquals(num, tolerance)) {
122117
return false;
123118
} else {
@@ -129,22 +124,22 @@ template <typename T> bool withinRelativeDifference(const T x, const T y, const
129124

130125
///@cond
131126
// Concrete instantiations
132-
template DLLExport bool equals<double>(const double, const double);
133-
template DLLExport bool equals<float>(const float, const float);
134-
template DLLExport bool ltEquals<double>(const double, const double);
135-
template DLLExport bool ltEquals<float>(const float, const float);
136-
template DLLExport bool gtEquals<double>(const double, const double);
137-
template DLLExport bool gtEquals<float>(const float, const float);
127+
template DLLExport bool equals<double>(double const, double const);
128+
template DLLExport bool equals<float>(float const, float const);
129+
template DLLExport bool ltEquals<double>(double const, double const);
130+
template DLLExport bool ltEquals<float>(float const, float const);
131+
template DLLExport bool gtEquals<double>(double const, double const);
132+
template DLLExport bool gtEquals<float>(float const, float const);
138133
//
139-
template DLLExport double absoluteDifference<double>(const double, const double);
140-
template DLLExport float absoluteDifference<float>(const float, const float);
141-
template DLLExport double relativeDifference<double>(const double, const double);
142-
template DLLExport float relativeDifference<float>(const float, const float);
134+
template DLLExport double absoluteDifference<double>(double const, double const);
135+
template DLLExport float absoluteDifference<float>(float const, float const);
136+
template DLLExport double relativeDifference<double>(double const, double const);
137+
template DLLExport float relativeDifference<float>(float const, float const);
143138
//
144-
template DLLExport bool withinAbsoluteDifference<double>(const double, const double, const double);
145-
template DLLExport bool withinAbsoluteDifference<float>(const float, const float, const float);
146-
template DLLExport bool withinRelativeDifference<double>(const double, const double, const double);
147-
template DLLExport bool withinRelativeDifference<float>(const float, const float, const float);
139+
template DLLExport bool withinAbsoluteDifference<double>(double const, double const, double const);
140+
template DLLExport bool withinAbsoluteDifference<float>(float const, float const, float const);
141+
template DLLExport bool withinRelativeDifference<double>(double const, double const, double const);
142+
template DLLExport bool withinRelativeDifference<float>(float const, float const, float const);
148143
///@endcond
149144

150145
} // namespace Mantid::Kernel

Framework/Kernel/test/FloatingPointComparisonTest.h

Lines changed: 54 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,25 @@ class FloatingPointComparisonTest : public CxxTest::TestSuite {
3030
TS_ASSERT_EQUALS(Mantid::Kernel::equals(0.1, 1.0001 * tol), false);
3131
}
3232

33+
void test_with_NaN() {
34+
// everything compares false with an NaN
35+
constexpr double anan = std::numeric_limits<double>::quiet_NaN();
36+
constexpr double bnan = std::numeric_limits<double>::quiet_NaN();
37+
constexpr double real = 3.0;
38+
// equals
39+
TS_ASSERT_EQUALS(Mantid::Kernel::equals(anan, real), false);
40+
TS_ASSERT_EQUALS(Mantid::Kernel::equals(real, anan), false);
41+
TS_ASSERT_EQUALS(Mantid::Kernel::equals(anan, bnan), false);
42+
// ltEquals
43+
TS_ASSERT_EQUALS(Mantid::Kernel::ltEquals(anan, real), false);
44+
TS_ASSERT_EQUALS(Mantid::Kernel::ltEquals(real, anan), false);
45+
TS_ASSERT_EQUALS(Mantid::Kernel::ltEquals(anan, bnan), false);
46+
// gtEquals
47+
TS_ASSERT_EQUALS(Mantid::Kernel::gtEquals(anan, real), false);
48+
TS_ASSERT_EQUALS(Mantid::Kernel::gtEquals(real, anan), false);
49+
TS_ASSERT_EQUALS(Mantid::Kernel::gtEquals(anan, bnan), false);
50+
}
51+
3352
void test_LtEquals_With_X_Equal_To_Y_Produces_True() {
3453
TS_ASSERT_EQUALS(Mantid::Kernel::ltEquals(0.1, 0.1), true);
3554
TS_ASSERT_EQUALS(Mantid::Kernel::ltEquals(-0.1, -0.1), true);
@@ -74,6 +93,11 @@ class FloatingPointComparisonTest : public CxxTest::TestSuite {
7493
TS_ASSERT_LESS_THAN(0.0, Mantid::Kernel::absoluteDifference(-left, -right));
7594
// test symmetry
7695
TS_ASSERT_EQUALS(Mantid::Kernel::absoluteDifference(left, right), Mantid::Kernel::absoluteDifference(right, left));
96+
// absolute difference with NaN is NaN
97+
constexpr double anan = std::numeric_limits<double>::quiet_NaN();
98+
constexpr double bnan = std::numeric_limits<double>::quiet_NaN();
99+
TS_ASSERT(std::isnan(Mantid::Kernel::absoluteDifference(left, anan)));
100+
TS_ASSERT(std::isnan(Mantid::Kernel::absoluteDifference(bnan, anan)));
77101
}
78102

79103
void test_relativeDifference() {
@@ -85,16 +109,21 @@ class FloatingPointComparisonTest : public CxxTest::TestSuite {
85109
TS_ASSERT_THROWS_NOTHING(Mantid::Kernel::relativeDifference(0.0, 0.0))
86110
TS_ASSERT(!std::isnan(Mantid::Kernel::relativeDifference(0.0, 0.0)));
87111
TS_ASSERT_EQUALS(Mantid::Kernel::relativeDifference(0.0, 0.0), 0.0);
88-
// check no errors using machine epsiolon
112+
// check no errors using machine epsilon
89113
constexpr double realsmall = std::numeric_limits<double>::epsilon();
90114
TS_ASSERT_THROWS_NOTHING(Mantid::Kernel::relativeDifference(0.0, realsmall))
91115
TS_ASSERT(!std::isnan(Mantid::Kernel::relativeDifference(0.0, realsmall)));
92116
TS_ASSERT_EQUALS(Mantid::Kernel::relativeDifference(0.0, realsmall), 0.0);
93117
// check we get correct values for normal situations
94-
constexpr double left = 2.6, right = 2.7;
95-
constexpr double reldiff = 2.0 * std::fabs(left - right) / (left + right);
118+
const double left = 2.6, right = 2.7;
119+
const double reldiff = 2.0 * std::abs(left - right) / (left + right);
96120
TS_ASSERT_EQUALS(Mantid::Kernel::relativeDifference(left, right), reldiff);
97121
TS_ASSERT_EQUALS(Mantid::Kernel::relativeDifference(right, left), reldiff);
122+
// relative difference with NaN is NaN
123+
constexpr double anan = std::numeric_limits<double>::quiet_NaN();
124+
constexpr double bnan = std::numeric_limits<double>::quiet_NaN();
125+
TS_ASSERT(std::isnan(Mantid::Kernel::relativeDifference(left, anan)));
126+
TS_ASSERT(std::isnan(Mantid::Kernel::absoluteDifference(bnan, anan)));
98127
}
99128

100129
void test_withinAbsoluteDifference() {
@@ -104,38 +133,48 @@ class FloatingPointComparisonTest : public CxxTest::TestSuite {
104133
TS_ASSERT_EQUALS(Mantid::Kernel::withinAbsoluteDifference(0.01, -0.011, 0.01), false);
105134
TS_ASSERT_EQUALS(Mantid::Kernel::withinAbsoluteDifference(100.1, 100.15, 0.1), true);
106135
TS_ASSERT_EQUALS(Mantid::Kernel::withinAbsoluteDifference(12345678923456.789, 12345679023456.788, 0.0001), false);
136+
// case of NaNs -- nothing is close to an NaN
107137
constexpr double anan = std::numeric_limits<double>::quiet_NaN();
108-
// TS_ASSERT_EQUALS( Mantid::Kernel::withinAbsoluteDifference(anan, 0.3, 0.1), false);
138+
constexpr double bnan = std::numeric_limits<double>::quiet_NaN();
139+
TS_ASSERT_EQUALS(Mantid::Kernel::withinAbsoluteDifference(anan, 0.3, 0.1), false);
140+
TS_ASSERT_EQUALS(Mantid::Kernel::withinAbsoluteDifference(anan, bnan, 0.1), false);
109141
}
110142

111143
void test_withinRelativeDifference() {
112-
// some cases with zero difference
113-
constexpr double point3 = 0.3, notquitepoint3 = 0.2 + 0.1;
144+
// things difference at machine epsilon are equal
145+
const double point3 = 0.3, notquitepoint3 = 0.2 + 0.1;
114146
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(point3, notquitepoint3, 1.e-307), true);
147+
// some cases with zero difference
115148
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(2.3, 2.3, 1.e-307), true);
116149
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(2.3e208, 2.3e208, 1.e-307), true);
150+
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(2.3e-208, 2.3e-208, 0.0), true);
117151
// case of large magnitude values -- even though abs diff would always fail, rel diff can still pass
118-
// passing
152+
// - passing
119153
TS_ASSERT_EQUALS(Mantid::Kernel::withinAbsoluteDifference(2.31e208, 2.32e208, 0.01), false);
120154
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(2.31e208, 2.32e208, 0.01), true);
121-
// failing
155+
// - failing
122156
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(2.3e208, 2.4e208, 0.01), false);
123157
// case of small magnitude values -- even though abs diff would always pass, rel diff still can fail
124-
// passing
158+
// - passing
125159
TS_ASSERT_EQUALS(Mantid::Kernel::withinAbsoluteDifference(2.31e-10, 2.32e-10, 0.01), true);
126160
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(2.31e-10, 2.32e-10, 0.01), true);
127-
// failing
161+
// - failing
128162
TS_ASSERT_EQUALS(Mantid::Kernel::withinAbsoluteDifference(2.3e-10, 2.4e-10, 0.01), true);
129163
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(2.3e-10, 2.4e-10, 0.01), false);
130164
// case of normal-sized values
131-
constexpr double left = 2.6, right = 2.7, far = 3.0;
132-
constexpr double reldiff = 2.0 * std::fabs(left - right) / (left + right);
133-
constexpr double tolerance = 1.01 * reldiff;
134-
// passing
165+
const double left = 2.6, right = 2.7, far = 3.0;
166+
const double reldiff = 2.0 * std::abs(left - right) / (left + right);
167+
const double tolerance = 1.01 * reldiff;
168+
// - passing
135169
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(left, right, tolerance), true);
136170
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(right, left, tolerance), true);
137-
// failing
171+
// - failing
138172
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(left, far, tolerance), false);
139173
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(far, left, tolerance), false);
174+
// case of NaNs -- nothing is close to an NaN
175+
constexpr double anan = std::numeric_limits<double>::quiet_NaN();
176+
constexpr double bnan = std::numeric_limits<double>::quiet_NaN();
177+
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(anan, 0.3, 0.1), false);
178+
TS_ASSERT_EQUALS(Mantid::Kernel::withinRelativeDifference(anan, bnan, 0.1), false);
140179
}
141180
};
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- add new functions for efficiently calculating absolute and relative differences

0 commit comments

Comments
 (0)