feat: common_unit selection algorithm improved to make rev + rad …

…return `rad`
mpusz · Oct 5, 2024 · cff9e25 · cff9e25
1 parent 21d07a4
commit cff9e25
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 7 deletions.
diff --git a/src/core/include/mp-units/framework/magnitude.h b/src/core/include/mp-units/framework/magnitude.h
@@ -190,6 +190,12 @@ template<typename T>
   return is_rational(element) && get_exponent(element).num > 0;
 }
 
+[[nodiscard]] consteval bool is_positive_integral_power(MagnitudeSpecExpr auto element)
+{
+  auto exp = get_exponent(element);
+  return exp.den == 1 && exp.num > 0;
+}
+
 ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 // Magnitude product implementation.
 [[nodiscard]] consteval bool less(MagnitudeSpecExpr auto lhs, MagnitudeSpecExpr auto rhs)
@@ -431,6 +437,10 @@ struct magnitude : detail::magnitude_base<magnitude<Ms...>> {
   // all below functions should in fact be in a `detail` namespace but are placed here to benefit from the ADL
   [[nodiscard]] friend consteval bool _is_integral(const magnitude&) { return (detail::is_integral(Ms) && ...); }
   [[nodiscard]] friend consteval bool _is_rational(const magnitude&) { return (detail::is_rational(Ms) && ...); }
+  [[nodiscard]] friend consteval bool _is_positive_integral_power(const magnitude&)
+  {
+    return (detail::is_positive_integral_power(Ms) && ...);
+  }
 
   /**
    * @brief  The value of a Magnitude in a desired type T.

diff --git a/src/core/include/mp-units/framework/unit.h b/src/core/include/mp-units/framework/unit.h
@@ -674,9 +674,9 @@ template<Unit U1, Unit U2>
     constexpr auto canonical_lhs = get_canonical_unit(U1{});
     constexpr auto canonical_rhs = get_canonical_unit(U2{});
 
-    if constexpr (_is_integral(canonical_lhs.mag / canonical_rhs.mag))
+    if constexpr (_is_positive_integral_power(canonical_lhs.mag / canonical_rhs.mag))
       return u2;
-    else if constexpr (_is_integral(canonical_rhs.mag / canonical_lhs.mag))
+    else if constexpr (_is_positive_integral_power(canonical_rhs.mag / canonical_lhs.mag))
       return u1;
     else {
       if constexpr (detail::unit_less<U1, U2>::value)

diff --git a/test/static/unit_symbol_test.cpp b/test/static/unit_symbol_test.cpp
@@ -211,9 +211,6 @@ static_assert(unit_symbol(get_common_unit(kilo<metre> / hour, metre / second) /
 static_assert(unit_symbol(get_common_unit(kilo<metre> / hour, metre / second) * second) ==
               "EQUIV{[1/5 km/h], [1/18 m/s]} s");
 static_assert(unit_symbol(get_common_unit(radian, degree)) == "EQUIV{[1/𝜋°], [1/180 rad]}");
-static_assert(unit_symbol(get_common_unit(angular::radian, angular::revolution)) == "EQUIV{rad, [2⁻¹ 𝜋⁻¹ rev]}");
-static_assert(unit_symbol<usf{.solidus = always}>(get_common_unit(angular::radian, angular::revolution)) ==
-              "EQUIV{rad, [1/(2 𝜋) rev]}");
 
 // derived units
 static_assert(unit_symbol(one) == "");  // NOLINT(readability-container-size-empty)

diff --git a/test/static/unit_test.cpp b/test/static/unit_test.cpp
@@ -540,8 +540,7 @@ static_assert(is_of_type<get_common_unit(mile, yard), yard_>);
 static_assert(
   is_of_type<get_common_unit(speed_of_light_in_vacuum, metre / second), derived_unit<metre_, per<second_>>>);
 
-static_assert(is_of_type<get_common_unit(radian, revolution), common_unit<radian_, revolution_>>);
-static_assert(is_of_type<get_common_unit(radian, degree), common_unit<degree_, radian_>>);
+static_assert(is_of_type<get_common_unit(radian, revolution), radian_>);
 
 // those should return instantiations of the `common_unit` class template
 static_assert(is_of_type<get_common_unit(kilometre, mile), common_unit<kilo_<metre_>, mile_>>);
@@ -550,6 +549,7 @@ static_assert(is_of_type<get_common_unit(kilometre / hour, metre / second),
                          common_unit<decltype(kilometre / hour), decltype(metre / second)>>);
 static_assert(is_of_type<get_common_unit(metre / second, kilometre / hour),
                          common_unit<decltype(kilometre / hour), decltype(metre / second)>>);
+static_assert(is_of_type<get_common_unit(radian, degree), common_unit<degree_, radian_>>);
 
 static_assert(
   is_of_type<get_common_unit(mile, kilometre) / second, derived_unit<common_unit<kilo_<metre_>, mile_>, per<second_>>>);