Skip to content

Commit

Permalink
Fix ulp distance calculation for numbers with different signs
Browse files Browse the repository at this point in the history
This is a simplification of the fix proposed in #2152, with the
critical function split out so that it can be tested directly,
without having to go through the ULP matcher.

Closes #2152
  • Loading branch information
horenmar committed Jul 27, 2021
1 parent bf61a41 commit bc74fef
Show file tree
Hide file tree
Showing 19 changed files with 502 additions and 38 deletions.
5 changes: 5 additions & 0 deletions docs/matchers.md
Expand Up @@ -161,6 +161,11 @@ away from the `target` value. The short version of what this means
is that there is no more than `maxUlpDiff - 1` representeable floating
point numbers between the argument for matching and the `target` value.

**Important**: The WithinULP matcher requires the platform to use the
[IEEE-754](https://en.wikipedia.org/wiki/IEEE_754) representation for
floating point numbers.


`WithinRel` creates a matcher that accepts floating point numbers that
are _approximately equal_ with the `target` with tolerance of `eps.`
Specifically, it matches if
Expand Down
2 changes: 2 additions & 0 deletions src/CMakeLists.txt
Expand Up @@ -65,6 +65,7 @@ set(INTERNAL_HEADERS
${SOURCES_DIR}/internal/catch_errno_guard.hpp
${SOURCES_DIR}/internal/catch_exception_translator_registry.hpp
${SOURCES_DIR}/internal/catch_fatal_condition_handler.hpp
${SOURCES_DIR}/internal/catch_floating_point_helpers.hpp
${SOURCES_DIR}/internal/catch_unique_name.hpp
${SOURCES_DIR}/generators/catch_generator_exception.hpp
${SOURCES_DIR}/generators/catch_generators.hpp
Expand Down Expand Up @@ -160,6 +161,7 @@ set(IMPL_SOURCES
${SOURCES_DIR}/internal/catch_enum_values_registry.cpp
${SOURCES_DIR}/internal/catch_exception_translator_registry.cpp
${SOURCES_DIR}/internal/catch_fatal_condition_handler.cpp
${SOURCES_DIR}/internal/catch_floating_point_helpers.cpp
${SOURCES_DIR}/generators/internal/catch_generators_combined_tu.cpp
${SOURCES_DIR}/interfaces/catch_interfaces_combined_tu.cpp
${SOURCES_DIR}/interfaces/catch_interfaces_reporter.cpp
Expand Down
1 change: 1 addition & 0 deletions src/catch2/catch_all.hpp
Expand Up @@ -66,6 +66,7 @@
#include <catch2/internal/catch_errno_guard.hpp>
#include <catch2/internal/catch_exception_translator_registry.hpp>
#include <catch2/internal/catch_fatal_condition_handler.hpp>
#include <catch2/internal/catch_floating_point_helpers.hpp>
#include <catch2/internal/catch_lazy_expr.hpp>
#include <catch2/internal/catch_leak_detector.hpp>
#include <catch2/internal/catch_list.hpp>
Expand Down
32 changes: 32 additions & 0 deletions src/catch2/internal/catch_floating_point_helpers.cpp
@@ -0,0 +1,32 @@

// Copyright Catch2 Authors
// Distributed under the Boost Software License, Version 1.0.
// (See accompanying file LICENSE_1_0.txt or copy at
// https://www.boost.org/LICENSE_1_0.txt)

// SPDX-License-Identifier: BSL-1.0

#include <catch2/internal/catch_floating_point_helpers.hpp>

#include <cstring>

namespace Catch {
namespace Detail {

uint32_t convertToBits(float f) {
static_assert(sizeof(float) == sizeof(uint32_t), "Important ULP matcher assumption violated");
uint32_t i;
std::memcpy(&i, &f, sizeof(f));
return i;
}

uint64_t convertToBits(double d) {
static_assert(sizeof(double) == sizeof(uint64_t), "Important ULP matcher assumption violated");
uint64_t i;
std::memcpy(&i, &d, sizeof(d));
return i;
}

} // end namespace Detail
} // end namespace Catch

88 changes: 88 additions & 0 deletions src/catch2/internal/catch_floating_point_helpers.hpp
@@ -0,0 +1,88 @@

// Copyright Catch2 Authors
// Distributed under the Boost Software License, Version 1.0.
// (See accompanying file LICENSE_1_0.txt or copy at
// https://www.boost.org/LICENSE_1_0.txt)

// SPDX-License-Identifier: BSL-1.0
#ifndef CATCH_FLOATING_POINT_HELPERS_HPP_INCLUDED
#define CATCH_FLOATING_POINT_HELPERS_HPP_INCLUDED

#include <catch2/internal/catch_polyfills.hpp>

#include <cmath>
#include <cstdint>
#include <utility>
#include <limits>

namespace Catch {
namespace Detail {

uint32_t convertToBits(float f);
uint64_t convertToBits(double d);

} // end namespace Detail


/**
* Calculates the ULP distance between two floating point numbers
*
* The ULP distance of two floating point numbers is the count of
* valid floating point numbers representable between them.
*
* There are some exceptions between how this function counts the
* distance, and the interpretation of the standard as implemented.
* by e.g. `nextafter`. For this function it always holds that:
* * `(x == y) => ulpDistance(x, y) == 0` (so `ulpDistance(-0, 0) == 0`)
* * `ulpDistance(maxFinite, INF) == 1`
* * `ulpDistance(x, -x) == 2 * ulpDistance(x, 0)`
*
* \pre `!isnan( lhs )`
* \pre `!isnan( rhs )`
* \pre floating point numbers are represented in IEEE-754 format
*/
template <typename FP>
uint64_t ulpDistance( FP lhs, FP rhs ) {
assert( std::numeric_limits<FP>::is_iec559 &&
"ulpDistance assumes IEEE-754 format for floating point types" );
assert( !Catch::isnan( lhs ) &&
"Distance between NaN and number is not meaningful" );
assert( !Catch::isnan( rhs ) &&
"Distance between NaN and number is not meaningful" );

// We want X == Y to imply 0 ULP distance even if X and Y aren't
// bit-equal (-0 and 0), or X - Y != 0 (same sign infinities).
if ( lhs == rhs ) { return 0; }

// We need a properly typed positive zero for type inference.
static constexpr FP positive_zero{};

// We want to ensure that +/- 0 is always represented as positive zero
if ( lhs == positive_zero ) { lhs = positive_zero; }
if ( rhs == positive_zero ) { rhs = positive_zero; }

// If arguments have different signs, we can handle them by summing
// how far are they from 0 each.
if ( std::signbit( lhs ) != std::signbit( rhs ) ) {
return ulpDistance( std::abs( lhs ), positive_zero ) +
ulpDistance( std::abs( rhs ), positive_zero );
}

// When both lhs and rhs are of the same sign, we can just
// read the numbers bitwise as integers, and then subtract them
// (assuming IEEE).
uint64_t lc = Detail::convertToBits( lhs );
uint64_t rc = Detail::convertToBits( rhs );

// The ulp distance between two numbers is symmetric, so to avoid
// dealing with overflows we want the bigger converted number on the lhs
if ( lc < rc ) {
std::swap( lc, rc );
}

return lc - rc;
}

} // end namespace Catch

#endif // CATCH_FLOATING_POINT_HELPERS_HPP_INCLUDED
32 changes: 7 additions & 25 deletions src/catch2/matchers/catch_matchers_floating_point.cpp
Expand Up @@ -10,12 +10,12 @@
#include <catch2/internal/catch_polyfills.hpp>
#include <catch2/internal/catch_to_string.hpp>
#include <catch2/catch_tostring.hpp>
#include <catch2/internal/catch_floating_point_helpers.hpp>

#include <algorithm>
#include <cmath>
#include <cstdlib>
#include <cstdint>
#include <cstring>
#include <sstream>
#include <iomanip>
#include <limits>
Expand All @@ -24,20 +24,6 @@
namespace Catch {
namespace {

int32_t convert(float f) {
static_assert(sizeof(float) == sizeof(int32_t), "Important ULP matcher assumption violated");
int32_t i;
std::memcpy(&i, &f, sizeof(f));
return i;
}

int64_t convert(double d) {
static_assert(sizeof(double) == sizeof(int64_t), "Important ULP matcher assumption violated");
int64_t i;
std::memcpy(&i, &d, sizeof(d));
return i;
}

template <typename FP>
bool almostEqualUlps(FP lhs, FP rhs, uint64_t maxUlpDiff) {
// Comparison with NaN should always be false.
Expand All @@ -46,17 +32,10 @@ namespace {
return false;
}

auto lc = convert(lhs);
auto rc = convert(rhs);

if ((lc < 0) != (rc < 0)) {
// Potentially we can have +0 and -0
return lhs == rhs;
}
// This should also handle positive and negative zeros, infinities
const auto ulpDist = ulpDistance(lhs, rhs);

// static cast as a workaround for IBM XLC
auto ulpDiff = std::abs(static_cast<FP>(lc - rc));
return static_cast<uint64_t>(ulpDiff) <= maxUlpDiff;
return ulpDist <= maxUlpDiff;
}

#if defined(CATCH_CONFIG_GLOBAL_NEXTAFTER)
Expand Down Expand Up @@ -131,6 +110,9 @@ namespace Detail {
CATCH_ENFORCE(m_type == Detail::FloatingPointKind::Double
|| m_ulps < (std::numeric_limits<uint32_t>::max)(),
"Provided ULP is impossibly large for a float comparison.");
CATCH_ENFORCE( std::numeric_limits<double>::is_iec559,
"WithinUlp matcher only supports platforms with "
"IEEE-754 compatible floating point representation" );
}

#if defined(__clang__)
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Expand Up @@ -80,6 +80,7 @@ set(TEST_SOURCES
${SELF_TEST_DIR}/IntrospectiveTests/Clara.tests.cpp
${SELF_TEST_DIR}/IntrospectiveTests/CmdLine.tests.cpp
${SELF_TEST_DIR}/IntrospectiveTests/Details.tests.cpp
${SELF_TEST_DIR}/IntrospectiveTests/FloatingPoint.tests.cpp
${SELF_TEST_DIR}/IntrospectiveTests/GeneratorsImpl.tests.cpp
${SELF_TEST_DIR}/IntrospectiveTests/InternalBenchmark.tests.cpp
${SELF_TEST_DIR}/IntrospectiveTests/PartTracker.tests.cpp
Expand Down
3 changes: 3 additions & 0 deletions tests/SelfTest/Baselines/automake.sw.approved.txt
Expand Up @@ -24,6 +24,8 @@ Nor would this
:test-result: PASS #1954 - 7 arg template test case sig compiles - 1, 1, 1, 1, 1, 0, 0
:test-result: PASS #1954 - 7 arg template test case sig compiles - 5, 1, 1, 1, 1, 0, 0
:test-result: PASS #1954 - 7 arg template test case sig compiles - 5, 3, 1, 1, 1, 0, 0
:test-result: PASS #2152 - ULP checks between differently signed values were wrong - double
:test-result: PASS #2152 - ULP checks between differently signed values were wrong - float
:test-result: XFAIL #748 - captures with unexpected exceptions
:test-result: PASS #809
:test-result: PASS #833
Expand Down Expand Up @@ -282,6 +284,7 @@ Message from section two
:test-result: PASS classify_outliers
:test-result: PASS comparisons between const int variables
:test-result: PASS comparisons between int variables
:test-result: PASS convertToBits
:test-result: PASS empty tags are not allowed
:test-result: PASS erfc_inv
:test-result: PASS estimate_clock_resolution
Expand Down
15 changes: 15 additions & 0 deletions tests/SelfTest/Baselines/compact.sw.approved.txt
Expand Up @@ -80,6 +80,10 @@ PartTracker.tests.cpp:<line number>: passed: n for: 3
Misc.tests.cpp:<line number>: passed:
Misc.tests.cpp:<line number>: passed:
Misc.tests.cpp:<line number>: passed:
Matchers.tests.cpp:<line number>: passed: smallest_non_zero, WithinULP( -smallest_non_zero, 2 ) for: 0.0 is within 2 ULPs of -4.9406564584124654e-324 ([-1.4821969375237396e-323, 4.9406564584124654e-324])
Matchers.tests.cpp:<line number>: passed: smallest_non_zero, !WithinULP( -smallest_non_zero, 1 ) for: 0.0 not is within 1 ULPs of -4.9406564584124654e-324 ([-9.8813129168249309e-324, -0.0000000000000000e+00])
Matchers.tests.cpp:<line number>: passed: smallest_non_zero, WithinULP( -smallest_non_zero, 2 ) for: 0.0f is within 2 ULPs of -1.40129846e-45f ([-4.20389539e-45, 1.40129846e-45])
Matchers.tests.cpp:<line number>: passed: smallest_non_zero, !WithinULP( -smallest_non_zero, 1 ) for: 0.0f not is within 1 ULPs of -1.40129846e-45f ([-2.80259693e-45, -0.00000000e+00])
Exception.tests.cpp:<line number>: failed: unexpected exception with message: 'answer := 42' with 1 message: 'expected exception'
Exception.tests.cpp:<line number>: failed: unexpected exception with message: 'answer := 42'; expression was: thisThrows() with 1 message: 'expected exception'
Exception.tests.cpp:<line number>: passed: thisThrows() with 1 message: 'answer := 42'
Expand Down Expand Up @@ -586,6 +590,7 @@ Matchers.tests.cpp:<line number>: passed: 10.f, !WithinAbs( 11.f, 0.5f ) for: 10
Matchers.tests.cpp:<line number>: passed: -10.f, WithinAbs( -10.f, 0.5f ) for: -10.0f is within 0.5 of -10.0
Matchers.tests.cpp:<line number>: passed: -10.f, WithinAbs( -9.6f, 0.5f ) for: -10.0f is within 0.5 of -9.6000003815
Matchers.tests.cpp:<line number>: passed: 1.f, WithinULP( 1.f, 0 ) for: 1.0f is within 0 ULPs of 1.00000000e+00f ([1.00000000e+00, 1.00000000e+00])
Matchers.tests.cpp:<line number>: passed: -1.f, WithinULP( -1.f, 0 ) for: -1.0f is within 0 ULPs of -1.00000000e+00f ([-1.00000000e+00, -1.00000000e+00])
Matchers.tests.cpp:<line number>: passed: nextafter( 1.f, 2.f ), WithinULP( 1.f, 1 ) for: 1.0f is within 1 ULPs of 1.00000000e+00f ([9.99999940e-01, 1.00000012e+00])
Matchers.tests.cpp:<line number>: passed: 0.f, WithinULP( nextafter( 0.f, 1.f ), 1 ) for: 0.0f is within 1 ULPs of 1.40129846e-45f ([0.00000000e+00, 2.80259693e-45])
Matchers.tests.cpp:<line number>: passed: 1.f, WithinULP( nextafter( 1.f, 0.f ), 1 ) for: 1.0f is within 1 ULPs of 9.99999940e-01f ([9.99999881e-01, 1.00000000e+00])
Expand Down Expand Up @@ -2065,6 +2070,16 @@ Condition.tests.cpp:<line number>: passed: long_var == unsigned_char_var for: 1
Condition.tests.cpp:<line number>: passed: long_var == unsigned_short_var for: 1 == 1
Condition.tests.cpp:<line number>: passed: long_var == unsigned_int_var for: 1 == 1
Condition.tests.cpp:<line number>: passed: long_var == unsigned_long_var for: 1 == 1
FloatingPoint.tests.cpp:<line number>: passed: convertToBits( 0.f ) == 0 for: 0 == 0
FloatingPoint.tests.cpp:<line number>: passed: convertToBits( -0.f ) == ( 1ULL << 31 ) for: 2147483648 (0x<hex digits>)
==
2147483648 (0x<hex digits>)
FloatingPoint.tests.cpp:<line number>: passed: convertToBits( 0. ) == 0 for: 0 == 0
FloatingPoint.tests.cpp:<line number>: passed: convertToBits( -0. ) == ( 1ULL << 63 ) for: 9223372036854775808 (0x<hex digits>)
==
9223372036854775808 (0x<hex digits>)
FloatingPoint.tests.cpp:<line number>: passed: convertToBits( std::numeric_limits<float>::denorm_min() ) == 1 for: 1 == 1
FloatingPoint.tests.cpp:<line number>: passed: convertToBits( std::numeric_limits<double>::denorm_min() ) == 1 for: 1 == 1
Tag.tests.cpp:<line number>: passed: Catch::TestCaseInfo("", { "test with an empty tag", "[]" }, dummySourceLineInfo)
InternalBenchmark.tests.cpp:<line number>: passed: erfc_inv(1.103560) == Approx(-0.09203687623843015) for: -0.0920368762 == Approx( -0.0920368762 )
InternalBenchmark.tests.cpp:<line number>: passed: erfc_inv(1.067400) == Approx(-0.05980291115763361) for: -0.0598029112 == Approx( -0.0598029112 )
Expand Down
4 changes: 2 additions & 2 deletions tests/SelfTest/Baselines/console.std.approved.txt
Expand Up @@ -1386,6 +1386,6 @@ due to unexpected exception with message:
Why would you throw a std::string?

===============================================================================
test cases: 365 | 289 passed | 70 failed | 6 failed as expected
assertions: 2092 | 1940 passed | 129 failed | 23 failed as expected
test cases: 368 | 292 passed | 70 failed | 6 failed as expected
assertions: 2103 | 1951 passed | 129 failed | 23 failed as expected

0 comments on commit bc74fef

Please sign in to comment.