* Fix incorrect transcedental function output for scaled dimensionless types
As reported in issue #284, some functions produce incorrect results when
passed certain dimensionless quantites. For example, this program from
the issue, slightly modified for brevity and to try to make a later
point clearer:
#include "units.h"
#include <iostream>
#include <cmath>
using namespace units::literals;
int main()
{
const auto c = 1.0_um * (-1 / 1.0_m);
std::cout << "c : " << c << std::endl ;
std::cout << "units::math::exp(c) : " << units::math::exp(c) << std::endl ;
std::cout << "std::exp(c.value()) : " << std::exp(c.value()) << std::endl ;
return EXIT_SUCCESS ;
}
Outputs:
c : -1e-06
units::math::exp(c) : 0.367879
std::exp(c.value()) : 0.999999
value() is basically to<>(), but with the template parameter hard-coded
to underlying_type, and to<>() is one of the documented ways to pass
something to a non-unit-enabled API, so getting different results this
way is rather surprising.
The proximate cause of this difference is the use of operator()()
instead of value() or to<>() by (some?) functions in units::math. In the
case of the example above:
template<class ScalarUnit>
dimensionless::scalar_t exp(const ScalarUnit x) noexcept
{
static_assert(traits::is_dimensionless_unit<ScalarUnit>::value, "Type `ScalarUnit` must be a dimensionless unit derived from `unit_t`.");
return dimensionless::scalar_t(std::exp(x()));
// operator()() instead of value()/to() ^^
}
The use of operator()() means that std::exp() is fed a different input
than when value() is used, as demonstrated by the output from an
appropriately modified version of the example program:
c : -1e-06
c() : -1
c.value() : -1e-06
units::math::exp(c) : 0.367879
std::exp(c.value()) : 0.999999
However, this problem does not for all possible inputs. If the
expression for c is changed as such:
// const auto c = 1.0_um * (-1 / 1.0_m);
const auto c = -1.0_um / 1.0_m;
The issue appears to vanish:
c : -1e-06
c() : -1e-06
c.value() : -1e-06
units::math::exp(c) : 0.999999
std::exp(c.value()) : 0.999999
I believe this is because the different calculations result in objects
with different types:
# const auto c = 1.0_um * (-1 / 1.0_m);
(lldb) p c
(const units::unit_t<units::unit<std::ratio<1, 1000000>, units::base_unit<std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1> >, std::ratio<0, 1>, std::ratio<0, 1> >, double, units::linear_scale>) $0 = {
units::linear_scale<double> = (m_value = -1)
}
# const auto c = 1.0_um / 1.0_m;
(lldb) p c
(const units::dimensionless::scalar_t) $0 = {
units::linear_scale<double> = (m_value = 9.9999999999999995E-7)
}
Where the latter is equivalent to:
# const auto c = 1.0_um / 1.0_m;
(lldb) p c
(const units::unit_t<units::unit<std::ratio<1, 1>, units::base_unit<std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1>, std::ratio<0, 1> >, std::ratio<0, 1>, std::ratio<0, 1> >, double, units::linear_scale>) $0 = {
units::linear_scale<double> = (m_value = 9.9999999999999995E-7)
}
The difference in the results amounts to the "new" calculation having
the "correct" value directly in m_value, whereas the "old" calculation
has some information in the type, which could be described as the
exponent in the type and the mantissa in m_value. Floating-point errors
aside, they are equal.
This points to the ultimate cause of the issue: operator()() and
value()/to<>() appear to consider different information when producing
their results. operator()() fully discards information encoded in the
type, directly returning the "raw" underlying value (modified for the
decibel scale if needed), while for dimensionless types value() and
to<>() account for both information encoded in the type and the "raw"
underlying value in their return values. This is apparent in their
implementations, where operator()() has no awareness of the units::unit
tag, but value()/to<>() delegate to the conversion operator for
dimensionless quantities, which explicitly "normalizes" the underlying
value using units::convert<>() before returning it:
template<class Units, typename T = UNIT_LIBDEFAULT_TYPE, template<typename> class NonLinearScale = linear_scale>
class unit_t : public NonLinearScale<T>, units::detail::_unit_t
{
public:
typedef T underlying_type;
inline constexpr underlying_type value() const noexcept
{
return static_cast<underlying_type>(*this);
}
template<typename Ty, class = std::enable_if_t<std::is_arithmetic<Ty>::value>>
inline constexpr Ty to() const noexcept
{
return static_cast<Ty>(*this);
}
template<class Ty, std::enable_if_t<traits::is_dimensionless_unit<Units>::value && std::is_arithmetic<Ty>::value, int> = 0>
inline constexpr operator Ty() const noexcept
{
// this conversion also resolves any PI exponents, by converting from a non-zero PI ratio to a zero-pi ratio.
return static_cast<Ty>(units::convert<Units, unit<std::ratio<1>, units::category::scalar_unit>>((*this)()));
}
};
template<typename T>
struct linear_scale
{
inline constexpr T operator()() const noexcept { return m_value; }
T m_value;
};
template<typename T>
struct decibel_scale
{
inline constexpr T operator()() const noexcept { return 10 * std::log10(m_value); }
T m_value;
};
As a result, the problem is clear: the functions using operator()() to
obtain a value to pass to std::math functions are incorrect and should
be changed to use value() so relevant information is not thrown away.
The functions changed in this commit are the functions grouped under the
"TRANSCEDENTAL FUNCTIONS" header in the header file. This is why modf()
is changed here despite not being a transcedental function. Some other
functions suffer from a similar issue, but those will be addressed in
different commits.
* Fix incorrect trig function output for scaled dimensionless types
Similar to the previous commit, but for certain trigonometric functions.
I don't think that explicitly using value() is strictly necessary for
the other trig functions since they already use convert<>() to
normalize, so I don't think normalization through value() is necessary.
I have not tested it, though.
Not entirely sure the modifications to the test cases are the best way
to make the desired changes. The idea is to use the same inputs/outputs,
but to change the unit type to have a conversion factor not equal to 1.
Co-authored-by: Alex Wang <ts826848@gmail.com>
* Remove spaces from line endings...
* Add constexpr to return value of arithmetic operators.
* Drop support for gcc-4.x and Visual Studio 14 because they not fully support C++14 standard.
Co-authored-by: Nic Holthaus <nholthaus@gmail.com>
* added compound assignment operators: +=, -=, *=, /=. Closes#44.
* print undefined unit types in SI base units. Closes#38. Update constants to NIST 2014 CODATA recommendations.
* Reduce metaprogramming boilerplate
* workaround for vs2013 internal compiler error
* added data and data transfer units. Closes#46.
* update doxyfile.in to v2.2.0
* add constexpr std::array test
* add julian and gregorian year definitions. Closes#29.
* added `value()` and `value_type` trait. Closes#56.
* fixes constexpr relational operator bug. Closes#63.
* improve compile time.
* removed some debug code.
* measure compile time
* Friend the linear scale units. Reduces compile time.
* add inline to pow() and abs() for being able to use units.h in different compilation units
* Revert "Friend the linear scale units. Reduces compile time."
This reverts commit e26fcd7d9b.
* Revert "add inline to pow() and abs() for being able to use units.h in different compilation units"
This reverts commit d42020e1de.
* add inline to pow() and abs() for being able to use units.h in different compilation units
* Test with Visual Studio 2017
* Another attempt at using Visual Studio 2017 in appveyor
* Build with VS2017
* add MSVC2017 badge
* started min/max
* fixes#65, fixes#71, fixes#76, fixes#79, fixes#80. Adds documentation.
* fix for vs2013, gcc
* updated docs
* fix for vs2013
* remove gcc warnings, add prefined unit defines
* min/max shouldn't be constexpr
* fix cmake
* more cmake fixes
* numeric_limits namespace
* fixes#77.
* fixes#75 & documentation
* fixed merge issues
* for Morwenn
* Make the project work with both ninja and msvc generators on windows
* Fixes#106. Locale aware `to_string` conversions.
* Fixes#105. Unary operators. Also removes VS 2013 testing (and support)
* Fixes#87. Don't include `to_string()` when iostream is disabled.
* fix a mistake in the unary +/-
* Add constexpr name/abbreviation member functions which don't require iostream.
* Fixes for linux build
* added compound assignment operators: +=, -=, *=, /=. Closes#44.
* print undefined unit types in SI base units. Closes#38. Update constants to NIST 2014 CODATA recommendations.
* Reduce metaprogramming boilerplate
* workaround for vs2013 internal compiler error
* added data and data transfer units. Closes#46.
* update doxyfile.in to v2.2.0
* add constexpr std::array test
* add julian and gregorian year definitions. Closes#29.
* added `value()` and `value_type` trait. Closes#56.
* fixes constexpr relational operator bug. Closes#63.
* improve compile time.
* removed some debug code.
* measure compile time
* Friend the linear scale units. Reduces compile time.
* add inline to pow() and abs() for being able to use units.h in different compilation units
* Revert "Friend the linear scale units. Reduces compile time."
This reverts commit e26fcd7d9b.
* Revert "add inline to pow() and abs() for being able to use units.h in different compilation units"
This reverts commit d42020e1de.
* add inline to pow() and abs() for being able to use units.h in different compilation units
* Test with Visual Studio 2017
* Another attempt at using Visual Studio 2017 in appveyor
* Build with VS2017
* add MSVC2017 badge
* started min/max
* fixes#65, fixes#71, fixes#76, fixes#79, fixes#80. Adds documentation.
* fix for vs2013, gcc
* updated docs
* fix for vs2013
* remove gcc warnings, add prefined unit defines
* min/max shouldn't be constexpr
* fix cmake
* more cmake fixes
* numeric_limits namespace
* fixes#77.
* fixes#75 & documentation
* fixed merge issues
* for Morwenn
- adds an `INTERFACE` cmake library for easier consumption as a subproject
- Removes GNU dependencies
- Updates google test to 1.8.0 and substantially simplifies the google test build
- adds caching to our CI system to improve build speed
- Add options to avoid building the unit tests/documentation (this will be helpful for subdirectories)
