Skip to content

Commit

Permalink
Fix incorrect transcedental function output for scaled dimensionless …
Browse files Browse the repository at this point in the history
…types

This is a port of 47de749 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 <cmath>
    #include <iostream>
    #include <units.h>
    using namespace units::literals;
    int main()
    {
        const auto c = 5.0_m * (2.0 / 1000.0_mm);
        std::cout << "c.to<double>() : " << c.to<double>() << std::endl;
        std::cout << "c.value()      : " << c.value() << std::endl;
        return EXIT_SUCCESS;
    }

Produces the following output in the v2.x branch:

    c.to<double>() : 10
    c.value()      : 10

But produces the following output in the v3.x branch:

    c.to<double>() : 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::conversion_factor<std::ratio<1000, 1>, 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 nholthaus#284 is present in the v3.x
branch, even if to<>() is used instead of value(). This program:

    #include <cmath>
    #include <iostream>
    #include <units.h>
    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<double>()           : " << c.to<double>() << std::endl;
        std::cout << "units::exp(c)            : " << units::exp(c) << std::endl;
        std::cout << "std::exp(c.to<double>()) : " << std::exp(c.to<double>()) << std::endl;
        return EXIT_SUCCESS;
    }

Produces the following output:

    c                        : 10
    c.to<double>()           : 10
    units::exp(c)            : 1010.05
    std::exp(c.to<double>()) : 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<class dimensionlessUnit, std::enable_if_t<traits::is_dimensionless_unit_v<dimensionlessUnit>, int> = 0>
    detail::floating_point_promotion_t<dimensionlessUnit> 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<underlying_type>() instead of value() to
ensure information in the value's type is taken into account.

    template<class dimensionlessUnit, std::enable_if_t<traits::is_dimensionless_unit_v<dimensionlessUnit>, int> = 0>
    detail::floating_point_promotion_t<dimensionlessUnit> exp(const dimensionlessUnit x) noexcept
    {
        return std::exp(x.template to<typename dimensionlessUnit::underlying_type>());
    }

(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<double>()           : 10
    units::exp(c)            : 2.20265e+07
    std::exp(c.to<double>()) : 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<dimensionlessUnit>, 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<class dimensionlessUnit, std::enable_if_t<traits::is_dimensionless_unit_v<dimensionlessUnit>, int> = 0>
    dimensionless<detail::floating_point_promotion_t<typename dimensionlessUnit::underlying_type>> exp(
    {
        return std::exp(x.template to<typename dimensionlessUnit::underlying_type>());
    }

This results in the expected output:

    c                        : 10
    c.to<double>()           : 10
    units::exp(c)            : 22026.5
    std::exp(c.to<double>()) : 22026.5
  • Loading branch information
Alex Wang committed Oct 6, 2022
1 parent 9fda61d commit b9d574a
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 16 deletions.
40 changes: 24 additions & 16 deletions include/units/core.h
Original file line number Diff line number Diff line change
Expand Up @@ -3632,9 +3632,10 @@ namespace units
* error occurs
*/
template<class dimensionlessUnit, std::enable_if_t<traits::is_dimensionless_unit_v<dimensionlessUnit>, int> = 0>
detail::floating_point_promotion_t<dimensionlessUnit> exp(const dimensionlessUnit x) noexcept
dimensionless<detail::floating_point_promotion_t<typename dimensionlessUnit::underlying_type>> exp(
const dimensionlessUnit x) noexcept
{
return std::exp(x.value());
return std::exp(x.template to<typename dimensionlessUnit::underlying_type>());
}

/**
Expand All @@ -3647,9 +3648,10 @@ namespace units
* @returns Natural logarithm of x.
*/
template<class dimensionlessUnit, std::enable_if_t<traits::is_dimensionless_unit_v<dimensionlessUnit>, int> = 0>
detail::floating_point_promotion_t<dimensionlessUnit> log(const dimensionlessUnit x) noexcept
dimensionless<detail::floating_point_promotion_t<typename dimensionlessUnit::underlying_type>> log(
const dimensionlessUnit x) noexcept
{
return std::log(x.value());
return std::log(x.template to<typename dimensionlessUnit::underlying_type>());
}

/**
Expand All @@ -3661,9 +3663,10 @@ namespace units
* @returns Common logarithm of x.
*/
template<class dimensionlessUnit, std::enable_if_t<traits::is_dimensionless_unit_v<dimensionlessUnit>, int> = 0>
detail::floating_point_promotion_t<dimensionlessUnit> log10(const dimensionlessUnit x) noexcept
dimensionless<detail::floating_point_promotion_t<typename dimensionlessUnit::underlying_type>> log10(
const dimensionlessUnit x) noexcept
{
return std::log10(x.value());
return std::log10(x.template to<typename dimensionlessUnit::underlying_type>());
}

/**
Expand All @@ -3681,10 +3684,11 @@ namespace units
std::enable_if_t<traits::is_dimensionless_unit_v<dimensionlessUnit> &&
std::is_floating_point_v<typename dimensionlessUnit::underlying_type>,
int> = 0>
dimensionlessUnit modf(const dimensionlessUnit x, dimensionlessUnit* intpart) noexcept
dimensionless<typename dimensionlessUnit::underlying_type> 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<decltype(intp)>(), &intp);
*intpart = intp;
return fracpart;
}
Expand All @@ -3697,9 +3701,10 @@ namespace units
* @returns 2 raised to the power of x.
*/
template<class dimensionlessUnit, std::enable_if_t<traits::is_dimensionless_unit_v<dimensionlessUnit>, int> = 0>
detail::floating_point_promotion_t<dimensionlessUnit> exp2(const dimensionlessUnit x) noexcept
dimensionless<detail::floating_point_promotion_t<typename dimensionlessUnit::underlying_type>> exp2(
const dimensionlessUnit x) noexcept
{
return std::exp2(x.value());
return std::exp2(static_cast<typename dimensionlessUnit::underlying_type>(x));
}

/**
Expand All @@ -3711,9 +3716,10 @@ namespace units
* @returns e raised to the power of x, minus one.
*/
template<class dimensionlessUnit, std::enable_if_t<traits::is_dimensionless_unit_v<dimensionlessUnit>, int> = 0>
detail::floating_point_promotion_t<dimensionlessUnit> expm1(const dimensionlessUnit x) noexcept
dimensionless<detail::floating_point_promotion_t<typename dimensionlessUnit::underlying_type>> expm1(
const dimensionlessUnit x) noexcept
{
return std::expm1(x.value());
return std::expm1(static_cast<typename dimensionlessUnit::underlying_type>(x));
}

/**
Expand All @@ -3726,9 +3732,10 @@ namespace units
* @returns The natural logarithm of (1+x).
*/
template<class dimensionlessUnit, std::enable_if_t<traits::is_dimensionless_unit_v<dimensionlessUnit>, int> = 0>
detail::floating_point_promotion_t<dimensionlessUnit> log1p(const dimensionlessUnit x) noexcept
dimensionless<detail::floating_point_promotion_t<typename dimensionlessUnit::underlying_type>> log1p(
const dimensionlessUnit x) noexcept
{
return std::log1p(x.value());
return std::log1p(static_cast<typename dimensionlessUnit::underlying_type>(x));
}

/**
Expand All @@ -3740,9 +3747,10 @@ namespace units
* @returns The binary logarithm of x: log2x.
*/
template<class dimensionlessUnit, std::enable_if_t<traits::is_dimensionless_unit_v<dimensionlessUnit>, int> = 0>
detail::floating_point_promotion_t<dimensionlessUnit> log2(const dimensionlessUnit x) noexcept
dimensionless<detail::floating_point_promotion_t<typename dimensionlessUnit::underlying_type>> log2(
const dimensionlessUnit x) noexcept
{
return std::log2(x.value());
return std::log2(static_cast<typename dimensionlessUnit::underlying_type>(x));
}

//----------------------------------
Expand Down
19 changes: 19 additions & 0 deletions unitTests/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3914,18 +3914,25 @@ TEST_F(UnitMath, exp)
{
double val = 10.0;
EXPECT_EQ(std::exp(val), exp(dimensionless<double>(val)));
auto uval = 5.0_m * (2.0 / 1000.0_mm);
EXPECT_EQ(static_cast<double>(uval), static_cast<typename decltype(uval)::underlying_type>(uval));
EXPECT_EQ(std::exp(uval.to<double>()), units::exp(uval));
}

TEST_F(UnitMath, log)
{
double val = 100.0;
EXPECT_EQ(std::log(val), log(dimensionless<double>(val)));
auto uval = 5.0_m * (2.0 / 1000.0_mm);
EXPECT_EQ(std::log(uval.to<double>()), units::log(uval));
}

TEST_F(UnitMath, log10)
{
double val = 100.0;
EXPECT_EQ(std::log10(val), log10(dimensionless<double>(val)));
auto uval = 5.0_m * (2.0 / 1000.0_mm);
EXPECT_EQ(std::log10(uval.to<double>()), units::log10(uval));
}

TEST_F(UnitMath, modf)
Expand All @@ -3935,30 +3942,42 @@ TEST_F(UnitMath, modf)
dimensionless<double> modfr2;
EXPECT_EQ(std::modf(val, &modfr1), modf(dimensionless<double>(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<double>(), &umodfr1), units::modf(uval, &umodfr2));
}

TEST_F(UnitMath, exp2)
{
double val = 10.0;
EXPECT_EQ(std::exp2(val), exp2(dimensionless<double>(val)));
auto uval = 5.0_m * (2.0 / 1000.0_mm);
EXPECT_EQ(std::exp2(uval.to<double>()), units::exp2(uval));
}

TEST_F(UnitMath, expm1)
{
double val = 10.0;
EXPECT_EQ(std::expm1(val), expm1(dimensionless<double>(val)));
auto uval = 5.0_m * (2.0 / 1000.0_mm);
EXPECT_EQ(std::expm1(uval.to<double>()), units::expm1(uval));
}

TEST_F(UnitMath, log1p)
{
double val = 10.0;
EXPECT_EQ(std::log1p(val), log1p(dimensionless<double>(val)));
auto uval = 5.0_m * (2.0 / 1000.0_mm);
EXPECT_EQ(std::log1p(uval.to<double>()), units::log1p(uval));
}

TEST_F(UnitMath, log2)
{
double val = 10.0;
EXPECT_EQ(std::log2(val), log2(dimensionless<double>(val)));
auto uval = 5.0_m * (2.0 / 1000.0_mm);
EXPECT_EQ(std::log2(uval.to<double>()), units::log2(uval));
}

TEST_F(UnitMath, pow)
Expand Down

0 comments on commit b9d574a

Please sign in to comment.