Skip to content

Commit

Permalink
Fix multiple inconsistencies with edge cases
Browse files Browse the repository at this point in the history
  • Loading branch information
Rinzii committed Feb 26, 2024
1 parent 91993f6 commit e27d64f
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 70 deletions.
1 change: 1 addition & 0 deletions ccmath_headers.cmake
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
set(ccmath_internal_helpers_headers
${CMAKE_CURRENT_SOURCE_DIR}/include/ccmath/internal/helpers/endian.hpp
${CMAKE_CURRENT_SOURCE_DIR}/include/ccmath/internal/helpers/narrow_cast.hpp
${CMAKE_CURRENT_SOURCE_DIR}/include/ccmath/internal/helpers/promote.hpp
)

set(ccmath_internal_typetraits_headers
Expand Down
109 changes: 48 additions & 61 deletions include/ccmath/detail/basic/fmod.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,109 +11,92 @@
//#include <type_traits>
#include <limits>



#include "ccmath/detail/compare/isnan.hpp"
#include "ccmath/detail/compare/isfinite.hpp"
#include "ccmath/detail/nearest/trunc.hpp"
#include "ccmath/detail/compare/signbit.hpp"

#include <cmath>


namespace ccm
{
namespace
{
namespace impl
{
template <typename T>
inline constexpr T fmod_impl_internal(T x, T y)
template <typename T, std::enable_if_t<std::is_floating_point_v<T>, int> = 0>
inline constexpr T fmod_impl_calculate(T x, T y)
{
// Calculate the remainder of the division of x by y.
return x - (ccm::trunc(x / y) * y);
return x - (ccm::trunc<T>(x / y) * y);
}

template <typename T, std::enable_if_t<std::is_integral_v<T>, int> = 0>
inline constexpr T fmod_impl_calculate(T x, T y)
{
// Calculate the remainder of the division of x by y.
// static_cast is required to prevent the compiler from complaining about narrowing with integer types.
return static_cast<T>(x - (ccm::trunc<T>(x / y) * y));
}

// Special case for floating point types
template <typename Real, std::enable_if_t<std::is_floating_point_v<Real>, bool> = true>
inline constexpr Real fmod_impl_switch(Real x, Real y) noexcept
template <typename T>
inline constexpr T fmod_impl_check(T x, T y) noexcept
{
// NOTE: We do not raise FE_INVALID even is situations where the standard demands it.
if constexpr (std::numeric_limits<Real>::is_iec559)
if constexpr (std::numeric_limits<T>::is_iec559)
{
// If x is ±0 and y is not zero, ±0 is returned.
if ((x == static_cast<Real>(0.0) || static_cast<Real>(x) == static_cast<Real>(-0.0)) && (y != static_cast<Real>(0.0)))
if ((x == static_cast<T>(0.0) || static_cast<T>(x) == static_cast<T>(-0.0)) && (y != static_cast<T>(0.0)))
{
// The standard specifies that plus or minus 0 is returned depending on the sign of x.
if (ccm::signbit(x))
{
return -Real{0.0};
return -static_cast<T>(0.0);
}
else
{
return Real{0.0};
return static_cast<T>(0.0);
}
}

// If x is ±∞ and y is not NaN, NaN is returned.
if ((x == +std::numeric_limits<Real>::infinity() || x == -std::numeric_limits<Real>::infinity()) && !ccm::isnan(y))
if ((x == +std::numeric_limits<T>::infinity() || x == -std::numeric_limits<T>::infinity()) && !ccm::isnan(y))
{
// The standard specifies that we always return the same sign as x.
if (ccm::signbit(x))
{
return -std::numeric_limits<Real>::quiet_NaN();
}
else
{
return std::numeric_limits<Real>::quiet_NaN();
}
// For some reason all the major compilers return a negative NaN even though I can't find anywhere
// in the standard that specifies this. I'm going to follow suit and return a negative NaN for now.
// Overall, this has little effect on checking for NaN. We only really care for conformance with the standard.
return -std::numeric_limits<T>::quiet_NaN();
}

// If y is ±0 and x is not NaN, NaN is returned.
if ((y == static_cast<Real>(0.0) || static_cast<Real>(y) == static_cast<Real>(-0.0)) && !ccm::isnan(x))
if ((y == static_cast<T>(0.0) || static_cast<T>(y) == static_cast<T>(-0.0)) && !ccm::isnan(x))
{
// The standard specifies that we always return the same sign as x.
if (ccm::signbit(x))
{
return -std::numeric_limits<Real>::quiet_NaN();
}
else
{
// NOTE: even though the standard specifies that we should return +NaN. On some compilers they return -NaN.
// We will return +NaN to be consistent with what is specified in the standard, but be
// aware that this edge case may be different from std::fmod.
return std::numeric_limits<Real>::quiet_NaN();
}
// Same issue as before. All major compilers return a negative NaN.
return -std::numeric_limits<T>::quiet_NaN();
}

// If y is ±∞ and x is finite, x is returned.
if ((y == +std::numeric_limits<Real>::infinity() || y == -std::numeric_limits<Real>::infinity()) && ccm::isfinite(x))
if ((y == +std::numeric_limits<T>::infinity() || y == -std::numeric_limits<T>::infinity()) && ccm::isfinite(x))
{
return x;
}

// If either argument is NaN, NaN is returned.
if (ccm::isnan(x) || ccm::isnan(y))
{
if (ccm::signbit(x)) // If x is negative then return negative NaN
{
return -std::numeric_limits<Real>::quiet_NaN();
}
else
{
return std::numeric_limits<Real>::quiet_NaN();
}
// Same problem as before but this time all major compilers return a positive NaN.
return std::numeric_limits<T>::quiet_NaN();
}
}

return fmod_impl_internal(x, y);
return fmod_impl_calculate(x, y);
}

template <typename Integer, std::enable_if_t<std::is_integral_v<Integer>, bool> = true>
inline constexpr double fmod_impl_switch(Integer x, Integer y) noexcept
template <typename T, typename U, typename TC = std::common_type_t<T, U>>
inline constexpr TC fmod_impl_type_check(T x, U y) noexcept
{
return fmod_impl_switch<double>(static_cast<double>(x), static_cast<double>(y));
return fmod_impl_check(static_cast<TC>(x), static_cast<TC>(y));
}

}
}

Expand All @@ -125,12 +108,24 @@ namespace ccm
* @param y A floating-point value.
* @return The floating-point remainder of the division operation x/y.
*/
template <typename T>
inline constexpr T fmod(T x, T y)
template <typename Real, std::enable_if_t<std::is_floating_point_v<Real>, int> = 0>
inline constexpr Real fmod(Real x, Real y)
{
return impl::fmod_impl_switch(x, y);
return impl::fmod_impl_check(x, y);
}

template <typename Integer, std::enable_if_t<std::is_integral_v<Integer>, int> = 0>
inline constexpr double fmod(Integer x, Integer y)
{
return impl::fmod_impl_type_check(x, y);
}

template <typename T, typename U>
inline constexpr std::common_type_t<T, U> fmod(T x, T y)
{
return impl::fmod_impl_type_check(x, y);
}

inline constexpr float fmodf(float x, float y)
{
return fmod<float>(x, y);
Expand All @@ -141,12 +136,4 @@ namespace ccm
return fmod<long double>(x, y);
}









} // namespace ccm
10 changes: 5 additions & 5 deletions include/ccmath/detail/nearest/trunc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

#include "ccmath/detail/basic/abs.hpp"
#include "ccmath/detail/compare/isnan.hpp"
#include "ccmath/internal/helpers/narrow_cast.hpp"
//#include "ccmath/internal/helpers/narrow_cast.hpp"

#include <limits>
#include <type_traits>
Expand Down Expand Up @@ -45,7 +45,7 @@ namespace ccm
}
}

return internal::narrow_cast<T>(static_cast<long long>(x));
return static_cast<T>(static_cast<long long>(x));
}
} // namespace impl
} // namespace
Expand All @@ -57,8 +57,8 @@ namespace ccm
* @param x The value to truncate.
* @return Returns a truncated value.
*/
template <typename T>
inline constexpr T trunc(T x) noexcept
template <typename Real, std::enable_if_t<std::is_floating_point_v<Real>, int> = 0>
inline constexpr Real trunc(Real x) noexcept
{
return impl::trunc_impl(x);
}
Expand All @@ -72,7 +72,7 @@ namespace ccm
template <typename Integer, std::enable_if_t<std::is_integral<Integer>::value, int> = 0>
inline constexpr double trunc(Integer x) noexcept
{
return internal::narrow_cast<double>(x);
return static_cast<double>(x);
}

/**
Expand Down
68 changes: 68 additions & 0 deletions include/ccmath/internal/helpers/promote.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright (c) 2024-Present Ian Pike
* Copyright (c) 2024-Present ccmath contributors
*
* This library is provided under the MIT License.
* See LICENSE for more information.
*/

#pragma once

#include <type_traits>

namespace ccm::helper
{
template <typename T, bool = std::is_integral_v<T>>
struct promote
{
using type = double;
};

template<typename T>
struct promote<T, false>
{ };

template<>
struct promote<long double, false>
{
using type = long double;
};

template<>
struct promote<double, false>
{
using type = double;
};

template<>
struct promote<float, false>
{
using type = float;
};

template<typename... T>
using promote_t = decltype((typename promote<T>::type(0) + ...)); // Assume we have fold expression

// Deducing the promoted type is done by promoted_t<T...>,
// then promote is used to provide the nested type member.
template <typename T, typename U>
using promote_2 = promote<promote_t<T, U>>;

template <typename T, typename U, typename V>
using promote_3 = promote<promote_t<T, U, V>>;

template <typename T, typename U, typename V, typename W>
using promote_4 = promote<promote_t<T, U, V, W>>;

template <typename T, typename U>
using promote_2_t = typename promote_2<T, U>::type;

template <typename T, typename U, typename V>
using promote_3_t = typename promote_3<T, U, V>::type;

template <typename T, typename U, typename V, typename W>
using promote_4_t = typename promote_4<T, U, V, W>::type;



}
54 changes: 50 additions & 4 deletions test/basic/fmod_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,65 @@
#include <ccmath/detail/basic/fmod.hpp>
#include <cmath>
#include <limits>
#include <type_traits>

namespace tes
{

}


TEST(CcmathBasicTests, Fmod)
{


// Test fmod with floating point numbers
EXPECT_FLOAT_EQ(ccm::fmod(10.0f, 3.0f), std::fmod(10.0f, 3.0f)); // NOLINT
EXPECT_FLOAT_EQ(ccm::fmod(10.0f, 3.0f), std::fmod(10.0f, 3.0f));
EXPECT_FLOAT_EQ(ccm::fmod(10.0f, -3.0f), std::fmod(10.0f, -3.0f));
EXPECT_FLOAT_EQ(ccm::fmod(-10.0f, 3.0f), std::fmod(-10.0f, 3.0f));
EXPECT_FLOAT_EQ(ccm::fmod(-10.0f, -3.0f), std::fmod(-10.0f, -3.0f));
//EXPECT_FLOAT_EQ(ccm::fmod(10.0f, 0.0f), std::fmod(10.0f, 0.0f)); // Not testable due to implementation defined behavior
EXPECT_FLOAT_EQ(ccm::fmod(0.0f, 3.0f), std::fmod(0.0f, 3.0f));
EXPECT_FLOAT_EQ(ccm::fmod(0.0f, 0.0f), std::fmod(0.0f, 0.0f));
EXPECT_FLOAT_EQ(ccm::fmod(0.0f, 3.0f), std::fmod(0.0f, 3.0f));

// Test fmod with integer numbers
EXPECT_FLOAT_EQ(ccm::fmod(10, 3), std::fmod(10, 3));


/// Test Edge Cases

// Test for edge case where if x is ±0 and y is not zero, ±0 is returned.
EXPECT_FLOAT_EQ(ccm::fmod(0.0f, 1.0f), std::fmod(0.0f, 1.0f));
EXPECT_FLOAT_EQ(ccm::fmod(-0.0f, 1.0f), std::fmod(-0.0f, 1.0f));

// Test for edge case where if x is ±∞ and y is not NaN, NaN is returned.
auto testForNanCcmIfXIsInfAndYIsNotNan = std::isnan(ccm::fmod(10.0f, 0.0f));
auto testForNanStdIfXIsInfAndYIsNotNan = std::isnan(std::fmod(10.0f, 0.0f));
bool isCcmNanSameAsStdNanIfXIsInfAndYIsNotNan = testForNanCcmIfXIsInfAndYIsNotNan == testForNanStdIfXIsInfAndYIsNotNan;
EXPECT_TRUE(isCcmNanSameAsStdNanIfXIsInfAndYIsNotNan);

// Test for edge case where if y is ±0 and x is not NaN, NaN is returned.
auto testForNanCcmIfYIsZeroAndXIsNotNan = std::isnan(ccm::fmod(10.0f, 0.0f));
auto testForNanStdIfYIsZeroAndXIsNotNan = std::isnan(std::fmod(10.0f, 0.0f));
bool isCcmNanSameAsStdNanIfYIsZeroAndXIsNotNan = testForNanCcmIfYIsZeroAndXIsNotNan == testForNanStdIfYIsZeroAndXIsNotNan;
EXPECT_TRUE(isCcmNanSameAsStdNanIfYIsZeroAndXIsNotNan);

// Test for edge case where if y is ±∞ and x is finite, x is returned.
EXPECT_FLOAT_EQ(ccm::fmod(10.0f, std::numeric_limits<float>::infinity()), std::fmod(10.0f, std::numeric_limits<float>::infinity()));
EXPECT_FLOAT_EQ(ccm::fmod(10.0f, -std::numeric_limits<float>::infinity()), std::fmod(10.0f, -std::numeric_limits<float>::infinity()));

// Test for edge case where if either argument is NaN, NaN is returned.
auto testForNanCcmIfEitherArgumentIsNan = std::isnan(ccm::fmod(std::numeric_limits<float>::quiet_NaN(), 10.0f));
auto testForNanStdIfEitherArgumentIsNan = std::isnan(std::fmod(std::numeric_limits<float>::quiet_NaN(), 10.0f));
bool isCcmNanSameAsStdNanIfEitherArgumentIsNan = testForNanCcmIfEitherArgumentIsNan == testForNanStdIfEitherArgumentIsNan;
EXPECT_TRUE(isCcmNanSameAsStdNanIfEitherArgumentIsNan);

testForNanCcmIfEitherArgumentIsNan = std::isnan(ccm::fmod(10.0f, std::numeric_limits<float>::quiet_NaN()));
testForNanStdIfEitherArgumentIsNan = std::isnan(std::fmod(10.0f, std::numeric_limits<float>::quiet_NaN()));
isCcmNanSameAsStdNanIfEitherArgumentIsNan = testForNanCcmIfEitherArgumentIsNan == testForNanStdIfEitherArgumentIsNan;
EXPECT_TRUE(isCcmNanSameAsStdNanIfEitherArgumentIsNan);






}

0 comments on commit e27d64f

Please sign in to comment.