From 2b37bc7b3e8c239a1d26cd614119b74d709ada2f Mon Sep 17 00:00:00 2001 From: Chip Hogg Date: Tue, 19 Apr 2022 15:42:43 +0000 Subject: [PATCH] Address review feedback --- src/core/include/units/magnitude.h | 26 +++++++++++++++++--------- src/core/include/units/quantity_cast.h | 22 ++++++++-------------- src/core/include/units/ratio.h | 4 ++-- test/unit_test/static/ratio_test.cpp | 6 ++++++ test/unit_test/static/si_fps_test.cpp | 18 ++++++++++++++---- 5 files changed, 47 insertions(+), 29 deletions(-) diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h index 60cd5b85..cde3a4b7 100644 --- a/src/core/include/units/magnitude.h +++ b/src/core/include/units/magnitude.h @@ -427,6 +427,18 @@ constexpr auto pow(magnitude) } } +template +constexpr auto sqrt(magnitude m) +{ + return pow(m); +} + +template +constexpr auto cbrt(magnitude m) +{ + return pow(m); +} + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // Magnitude product implementation. @@ -569,18 +581,14 @@ constexpr auto common_magnitude(magnitude m1, magnitude m2 { using detail::remove_positive_power; - // Case for when H1 has the smaller base. if constexpr (H1.get_base() < H2.get_base()) { + // When H1 has the smaller base, prepend to result from recursion. return remove_positive_power(magnitude

{}) * common_magnitude(magnitude{}, m2); - } - - // Case for when H2 has the smaller base. - if constexpr (H2.get_base() < H1.get_base()) { + } else if constexpr (H2.get_base() < H1.get_base()) { + // When H2 has the smaller base, prepend to result from recursion. return remove_positive_power(magnitude

{}) * common_magnitude(m1, magnitude{}); - } - - // Case for equal bases. - if constexpr (H1.get_base() == H2.get_base()) { + } else { + // When the bases are equal, pick whichever has the lower power. constexpr auto common_tail = common_magnitude(magnitude{}, magnitude{}); if constexpr (H1.power < H2.power) { return magnitude

{} * common_tail; diff --git a/src/core/include/units/quantity_cast.h b/src/core/include/units/quantity_cast.h index a2664339..8c40a87f 100644 --- a/src/core/include/units/quantity_cast.h +++ b/src/core/include/units/quantity_cast.h @@ -62,10 +62,7 @@ inline constexpr Magnitude auto quantity_magnitude> = [] { }(); template -inline constexpr ratio quantity_ratio = std::enable_if_t>{}; - -template -inline constexpr ratio quantity_ratio> = as_ratio(quantity_magnitude>); +inline constexpr ratio quantity_ratio = as_ratio(quantity_magnitude); template inline constexpr Magnitude auto cast_magnitude = [] { @@ -119,17 +116,14 @@ template R using traits = detail::cast_traits; using ratio_type = TYPENAME traits::ratio_type; using rep_type = TYPENAME traits::rep_type; - constexpr Magnitude auto c_mag = detail::cast_magnitude, To>; - if constexpr (treat_as_floating_point) { - return To( - static_cast(static_cast(q.number()) * (get_value(numerator(c_mag)) / - get_value(denominator(c_mag))))); - } else { - return To( - static_cast(static_cast(q.number()) * get_value(numerator(c_mag)) / - get_value(denominator(c_mag)))); - } + constexpr Magnitude auto c_mag = detail::cast_magnitude, To>; + constexpr Magnitude auto num = numerator(c_mag); + constexpr Magnitude auto den = denominator(c_mag); + constexpr Magnitude auto irr = c_mag * (den / num); + + constexpr auto val = [](Magnitude auto m) { return get_value(m); }; + return To(static_cast(static_cast(q.number()) * val(num) / val(den) * val(irr))); } /** diff --git a/src/core/include/units/ratio.h b/src/core/include/units/ratio.h index c5e03d5c..b57ce06d 100644 --- a/src/core/include/units/ratio.h +++ b/src/core/include/units/ratio.h @@ -61,6 +61,8 @@ struct ratio { [[nodiscard]] friend constexpr bool operator==(const ratio&, const ratio&) = default; + [[nodiscard]] friend constexpr auto operator<=>(const ratio& lhs, const ratio& rhs) { return (lhs - rhs).num <=> 0; } + [[nodiscard]] friend constexpr ratio operator-(const ratio& r) { return ratio(-r.num, r.den, r.exp); } [[nodiscard]] friend constexpr ratio operator+(ratio lhs, ratio rhs) @@ -81,8 +83,6 @@ struct ratio { [[nodiscard]] friend constexpr ratio operator-(const ratio& lhs, const ratio& rhs) { return lhs + (-rhs); } - [[nodiscard]] friend constexpr auto operator<=>(const ratio& lhs, const ratio& rhs) { return (lhs - rhs).num <=> 0; } - [[nodiscard]] friend constexpr ratio operator*(const ratio& lhs, const ratio& rhs) { const std::intmax_t gcd1 = std::gcd(lhs.num, rhs.den); diff --git a/test/unit_test/static/ratio_test.cpp b/test/unit_test/static/ratio_test.cpp index 4abe5094..ad966950 100644 --- a/test/unit_test/static/ratio_test.cpp +++ b/test/unit_test/static/ratio_test.cpp @@ -108,4 +108,10 @@ static_assert(numerator(ratio(3, 7, 2)) == 300); static_assert(denominator(ratio(3, 4)) == 4); static_assert(denominator(ratio(3, 7, -2)) == 700); +// comparison +static_assert((ratio(3, 4) <=> ratio(6, 8)) == (0 <=> 0)); +static_assert((ratio(3, 4) <=> ratio(-3, 4)) == (0 <=> -1)); +static_assert((ratio(-3, 4) <=> ratio(3, -4)) == (0 <=> 0)); +static_assert((ratio(1, 1, 1) <=> ratio(10)) == (0 <=> 0)); + } // namespace diff --git a/test/unit_test/static/si_fps_test.cpp b/test/unit_test/static/si_fps_test.cpp index a3b0aeb5..bb023c8c 100644 --- a/test/unit_test/static/si_fps_test.cpp +++ b/test/unit_test/static/si_fps_test.cpp @@ -31,6 +31,7 @@ #include #include #include +#include namespace { @@ -113,6 +114,13 @@ static_assert(1_q_pdl_per_ft2 > 1.4881639435_q_Pa && 1_q_pdl_per_ft2 < 1.4881639 } // namespace fps_plus_si_literals namespace fps_test { +namespace { +constexpr bool is_near(auto a, auto b, auto tol) +{ + const auto diff = a - b; + return (diff <= tol) && (-diff <= tol); +} +} // namespace using namespace units::isq::si::fps::literals; using namespace units::isq::si::fps::references; @@ -121,10 +129,12 @@ using namespace units::isq::si::fps::references; static_assert(si::length(1) + 1 * ft == si::length(1.3048)); static_assert(1 * ft + si::length(1) == si::length(1.3048)); -static_assert(quantity_cast>(1. * ft / 0.3048) + si::length(1) == - si::length(2)); // 1 m in ft + 1 m -static_assert(si::length(1) + quantity_cast>(1. * ft / 0.3048) == - si::length(2)); // 1 m + 1 m in ft +static_assert(is_near(quantity_cast>(1. * ft / 0.3048) + si::length(1), + si::length(2), + si::length(1))); // 1 m in ft + 1 m +static_assert(is_near(si::length(1) + quantity_cast>(1. * ft / 0.3048), + si::length(2), + si::length(1))); // 1 m + 1 m in ft static_assert(1 * ft + quantity_cast>(si::length(0.3048)) == 2 * ft); // 1 ft + 1 ft in m static_assert(quantity_cast>(si::length(0.3048)) + 1 * ft ==