From b9d574a5c077b7431332f705fc219c2adfef00d0 Mon Sep 17 00:00:00 2001 From: Alex Wang Date: Thu, 6 Oct 2022 14:57:16 -0400 Subject: [PATCH] Fix incorrect transcedental function output for scaled dimensionless types This is a port of 47de749f95b4908999dd326ff171522d56d669ba to the v3.x branch. The v3.x branch has some differences, though, which are detailed below. The first difference of note is that the implementation of value() has changed. Instead of essentially being to<>() with the template parameter hard-coded to underlying_type, value() now discards type information and returns the raw underlying value. For example, this program: #include #include #include using namespace units::literals; int main() { const auto c = 5.0_m * (2.0 / 1000.0_mm); std::cout << "c.to() : " << c.to() << std::endl; std::cout << "c.value() : " << c.value() << std::endl; return EXIT_SUCCESS; } Produces the following output in the v2.x branch: c.to() : 10 c.value() : 10 But produces the following output in the v3.x branch: c.to() : 10 c.value() : 0.01 The way that value() drops type information is apparent when inspecting the value of c under a debugger: (lldb) p c (const units::unit, units::dimension_t<>, std::ratio<0, 1>, std::ratio<0, 1> >, double, units::linear_scale>) $0 = (linearized_value = 0.01) In any case, the bug displayed in issue #284 is present in the v3.x branch, even if to<>() is used instead of value(). This program: #include #include #include using namespace units::literals; int main() { const auto c = 5.0_m * (2.0 / 1000.0_mm); std::cout << "c : " << c << std::endl; std::cout << "c.to() : " << c.to() << std::endl; std::cout << "units::exp(c) : " << units::exp(c) << std::endl; std::cout << "std::exp(c.to()) : " << std::exp(c.to()) << std::endl; return EXIT_SUCCESS; } Produces the following output: c : 10 c.to() : 10 units::exp(c) : 1010.05 std::exp(c.to()) : 22026.5 This leads into the second difference. The transcedental functions no longer use operator()(), which appears to have been removed; instead, they use value() to obtain the value to pass to the standard library math function: template, int> = 0> detail::floating_point_promotion_t exp(const dimensionlessUnit x) noexcept { return std::exp(x.value()); } As detailed above, the use of value() results in an incorrect value being passed to the underlying function. Fixing this is simple, albeit somewhat verbose - use to() instead of value() to ensure information in the value's type is taken into account. template, int> = 0> detail::floating_point_promotion_t exp(const dimensionlessUnit x) noexcept { return std::exp(x.template to()); } (This change can be simplified and/or obviated if value() is changed to not discard information in the type.) However, unlike in the v2.x branch, this change is not sufficient to obtain the expected results: c : 10 c.to() : 10 units::exp(c) : 2.20265e+07 std::exp(c.to()) : 22026.5 This appears to be due to the change to the return type. In the v2.x branch, the transcedental functions returned dimensionless::scalar_t, which I believe is equivalent to dimensionless<> in the v3.x branch. However, in the v3.x branch these functions all return floating_point_promotion_t, which is more or less the argument's unit with a (potentially) promoted underlying_type. This, however, happens to preserve the argument unit's conversion factor. When the conversion factor is not 1, this appears to result in the output of the underlying standard library function being scaled, resulting in an incorrect output. Changing the return type to be a promoted dimensionless type with a conversion factor of 1 appears to solve the issue, though it does add a fair amount of clutter: template, int> = 0> dimensionless> exp( { return std::exp(x.template to()); } This results in the expected output: c : 10 c.to() : 10 units::exp(c) : 22026.5 std::exp(c.to()) : 22026.5 --- include/units/core.h | 40 ++++++++++++++++++++++++---------------- unitTests/main.cpp | 19 +++++++++++++++++++ 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/include/units/core.h b/include/units/core.h index b3726884..c7a213b9 100644 --- a/include/units/core.h +++ b/include/units/core.h @@ -3632,9 +3632,10 @@ namespace units * error occurs */ template, int> = 0> - detail::floating_point_promotion_t exp(const dimensionlessUnit x) noexcept + dimensionless> exp( + const dimensionlessUnit x) noexcept { - return std::exp(x.value()); + return std::exp(x.template to()); } /** @@ -3647,9 +3648,10 @@ namespace units * @returns Natural logarithm of x. */ template, int> = 0> - detail::floating_point_promotion_t log(const dimensionlessUnit x) noexcept + dimensionless> log( + const dimensionlessUnit x) noexcept { - return std::log(x.value()); + return std::log(x.template to()); } /** @@ -3661,9 +3663,10 @@ namespace units * @returns Common logarithm of x. */ template, int> = 0> - detail::floating_point_promotion_t log10(const dimensionlessUnit x) noexcept + dimensionless> log10( + const dimensionlessUnit x) noexcept { - return std::log10(x.value()); + return std::log10(x.template to()); } /** @@ -3681,10 +3684,11 @@ namespace units std::enable_if_t && std::is_floating_point_v, int> = 0> - dimensionlessUnit modf(const dimensionlessUnit x, dimensionlessUnit* intpart) noexcept + dimensionless modf( + const dimensionlessUnit x, dimensionlessUnit* intpart) noexcept { typename dimensionlessUnit::underlying_type intp; - dimensionlessUnit fracpart = std::modf(x.value(), &intp); + dimensionlessUnit fracpart = std::modf(x.template to(), &intp); *intpart = intp; return fracpart; } @@ -3697,9 +3701,10 @@ namespace units * @returns 2 raised to the power of x. */ template, int> = 0> - detail::floating_point_promotion_t exp2(const dimensionlessUnit x) noexcept + dimensionless> exp2( + const dimensionlessUnit x) noexcept { - return std::exp2(x.value()); + return std::exp2(static_cast(x)); } /** @@ -3711,9 +3716,10 @@ namespace units * @returns e raised to the power of x, minus one. */ template, int> = 0> - detail::floating_point_promotion_t expm1(const dimensionlessUnit x) noexcept + dimensionless> expm1( + const dimensionlessUnit x) noexcept { - return std::expm1(x.value()); + return std::expm1(static_cast(x)); } /** @@ -3726,9 +3732,10 @@ namespace units * @returns The natural logarithm of (1+x). */ template, int> = 0> - detail::floating_point_promotion_t log1p(const dimensionlessUnit x) noexcept + dimensionless> log1p( + const dimensionlessUnit x) noexcept { - return std::log1p(x.value()); + return std::log1p(static_cast(x)); } /** @@ -3740,9 +3747,10 @@ namespace units * @returns The binary logarithm of x: log2x. */ template, int> = 0> - detail::floating_point_promotion_t log2(const dimensionlessUnit x) noexcept + dimensionless> log2( + const dimensionlessUnit x) noexcept { - return std::log2(x.value()); + return std::log2(static_cast(x)); } //---------------------------------- diff --git a/unitTests/main.cpp b/unitTests/main.cpp index 94cf17fc..4ffae7f5 100644 --- a/unitTests/main.cpp +++ b/unitTests/main.cpp @@ -3914,18 +3914,25 @@ TEST_F(UnitMath, exp) { double val = 10.0; EXPECT_EQ(std::exp(val), exp(dimensionless(val))); + auto uval = 5.0_m * (2.0 / 1000.0_mm); + EXPECT_EQ(static_cast(uval), static_cast(uval)); + EXPECT_EQ(std::exp(uval.to()), units::exp(uval)); } TEST_F(UnitMath, log) { double val = 100.0; EXPECT_EQ(std::log(val), log(dimensionless(val))); + auto uval = 5.0_m * (2.0 / 1000.0_mm); + EXPECT_EQ(std::log(uval.to()), units::log(uval)); } TEST_F(UnitMath, log10) { double val = 100.0; EXPECT_EQ(std::log10(val), log10(dimensionless(val))); + auto uval = 5.0_m * (2.0 / 1000.0_mm); + EXPECT_EQ(std::log10(uval.to()), units::log10(uval)); } TEST_F(UnitMath, modf) @@ -3935,30 +3942,42 @@ TEST_F(UnitMath, modf) dimensionless modfr2; EXPECT_EQ(std::modf(val, &modfr1), modf(dimensionless(val), &modfr2)); EXPECT_EQ(modfr1, modfr2); + auto uval = 5.0_m * (2.0 / 1000.0_mm); + double umodfr1; + decltype(uval) umodfr2; + EXPECT_EQ(std::modf(uval.to(), &umodfr1), units::modf(uval, &umodfr2)); } TEST_F(UnitMath, exp2) { double val = 10.0; EXPECT_EQ(std::exp2(val), exp2(dimensionless(val))); + auto uval = 5.0_m * (2.0 / 1000.0_mm); + EXPECT_EQ(std::exp2(uval.to()), units::exp2(uval)); } TEST_F(UnitMath, expm1) { double val = 10.0; EXPECT_EQ(std::expm1(val), expm1(dimensionless(val))); + auto uval = 5.0_m * (2.0 / 1000.0_mm); + EXPECT_EQ(std::expm1(uval.to()), units::expm1(uval)); } TEST_F(UnitMath, log1p) { double val = 10.0; EXPECT_EQ(std::log1p(val), log1p(dimensionless(val))); + auto uval = 5.0_m * (2.0 / 1000.0_mm); + EXPECT_EQ(std::log1p(uval.to()), units::log1p(uval)); } TEST_F(UnitMath, log2) { double val = 10.0; EXPECT_EQ(std::log2(val), log2(dimensionless(val))); + auto uval = 5.0_m * (2.0 / 1000.0_mm); + EXPECT_EQ(std::log2(uval.to()), units::log2(uval)); } TEST_F(UnitMath, pow)