fix: quantity scaling between different prefixes improved

Resolves #608

src/core/include/mp-units/bits/sudo_cast.h

@@ -31,35 +31,25 @@
 namespace mp_units::detail {
 
 template<typename T, typename Other>
-struct get_common_type : std::common_type<T, Other> {};
+using maybe_common_type = std::conditional_t<requires { typename std::common_type_t<T, Other>; },
-
+                                             std::common_type<T, Other>, std::type_identity<T>>::type;
-template<typename T, typename Other>
-using maybe_common_type = MP_UNITS_TYPENAME std::conditional_t<requires { typename std::common_type_t<T, Other>; },
-                                                               get_common_type<T, Other>, std::type_identity<T>>::type;
 
 /**
- * @brief Details about the conversion from one quantity to another.
+ * @brief Type-related details about the conversion from one quantity to another
  *
- * This struct calculates the conversion factor that needs to be applied to a number,
+ * This trait helps to determine what representations to use at which step in the conversion process,
- * in order to convert from one quantity to another. In addition to that, it also
- * helps to determine what representations to use at which step in the conversion process,
  * in order to avoid overflow and underflow while not causing excessive computations.
  *
  * @note This is a low-level facility.
  *
- * @tparam To a target quantity type to cast to
+ * @tparam M common magnitude between the two quantities
- * @tparam From a source quantity type to cast from
+ * @tparam Rep1 first quantity representation type
+ * @tparam Rep2 second quantity representation type
  */
-template<Quantity To, Quantity From>
+template<Magnitude auto M, typename Rep1, typename Rep2>
-  requires(castable(From::quantity_spec, To::quantity_spec))
+struct conversion_type_traits {
-struct magnitude_conversion_traits {
+  using c_rep_type = maybe_common_type<Rep1, Rep2>;
-  // scale the number
+  using c_mag_type = common_magnitude_type<M>;
-  static constexpr Magnitude auto c_mag = get_canonical_unit(From::unit).mag / get_canonical_unit(To::unit).mag;
-  static constexpr Magnitude auto num = numerator(c_mag);
-  static constexpr Magnitude auto den = denominator(c_mag);
-  static constexpr Magnitude auto irr = c_mag * (den / num);
-  using c_rep_type = maybe_common_type<typename std::remove_reference_t<From>::rep, typename To::rep>;
-  using c_mag_type = common_magnitude_type<c_mag>;
   using multiplier_type = conditional<
     treat_as_floating_point<c_rep_type>,
     // ensure that the multiplier is also floating-point
@@ -68,11 +58,28 @@ struct magnitude_conversion_traits {
                 std::common_type_t<c_mag_type, value_type_t<c_rep_type>>, std::common_type_t<c_mag_type, double>>,
     c_mag_type>;
   using c_type = maybe_common_type<c_rep_type, multiplier_type>;
-  static constexpr auto val(Magnitude auto m) { return get_value<multiplier_type>(m); };
+};
-  static constexpr multiplier_type num_mult = val(num);
+
-  static constexpr multiplier_type den_mult = val(den);
+/**
-  static constexpr multiplier_type irr_mult = val(irr);
+ * @brief Value-related details about the conversion from one quantity to another
-  static constexpr multiplier_type ratio = num_mult / den_mult * irr_mult;
+ *
+ * This trait provide ingredients to calculate the conversion factor that needs to be applied
+ * to a number, in order to convert from one quantity to another.
+ *
+ * @note This is a low-level facility.
+ *
+ * @tparam M common magnitude between the two quantities
+ * @tparam T common multiplier representation type
+ */
+template<Magnitude auto M, typename T>
+struct conversion_value_traits {
+  static constexpr Magnitude auto num = numerator(M);
+  static constexpr Magnitude auto den = denominator(M);
+  static constexpr Magnitude auto irr = M * (den / num);
+  static constexpr T num_mult = get_value<T>(num);
+  static constexpr T den_mult = get_value<T>(den);
+  static constexpr T irr_mult = get_value<T>(irr);
+  static constexpr T ratio = num_mult / den_mult * irr_mult;
 };
 
 
@@ -84,35 +91,43 @@ struct magnitude_conversion_traits {
  *
  * @tparam To a target quantity type to cast to
  */
-template<Quantity To, typename From>
+template<Quantity To, typename FwdFrom, typename From = std::remove_cvref_t<FwdFrom>>
-  requires Quantity<std::remove_cvref_t<From>> &&
+  requires Quantity<From> && (castable(From::quantity_spec, To::quantity_spec)) &&
-           (castable(std::remove_reference_t<From>::quantity_spec, To::quantity_spec)) &&
+           ((From::unit == To::unit && std::constructible_from<typename To::rep, typename From::rep>) ||
-           ((std::remove_reference_t<From>::unit == To::unit &&
+            (From::unit != To::unit))  // && scalable_with_<typename To::rep>))
-             std::constructible_from<typename To::rep, typename std::remove_reference_t<From>::rep>) ||
-            (std::remove_reference_t<From>::unit != To::unit))  // && scalable_with_<typename To::rep>))
 // TODO how to constrain the second part here?
-[[nodiscard]] constexpr To sudo_cast(From&& q)
+[[nodiscard]] constexpr To sudo_cast(FwdFrom&& q)
 {
-  constexpr auto q_unit = std::remove_reference_t<From>::unit;
+  constexpr auto q_unit = From::unit;
   if constexpr (q_unit == To::unit) {
     // no scaling of the number needed
-    return {static_cast<MP_UNITS_TYPENAME To::rep>(std::forward<From>(q).numerical_value_is_an_implementation_detail_),
+    return {static_cast<To::rep>(std::forward<FwdFrom>(q).numerical_value_is_an_implementation_detail_),
             To::reference};  // this is the only (and recommended) way to do a truncating conversion on a number, so we
                              // are using static_cast to suppress all the compiler warnings on conversions
   } else {
+    static constexpr Magnitude auto c_mag = get_canonical_unit(From::unit).mag / get_canonical_unit(To::unit).mag;
+    using type_traits = conversion_type_traits<c_mag, typename From::rep, typename To::rep>;
+    using multiplier_type = typename type_traits::multiplier_type;
+    auto scale = [&](std::invocable<typename type_traits::c_type> auto func) {
+      auto res =
+        static_cast<To::rep>(func(static_cast<type_traits::c_type>(q.numerical_value_is_an_implementation_detail_)));
+      return To{res, To::reference};
+    };
+
     // scale the number
-    using traits = magnitude_conversion_traits<To, std::remove_reference_t<From>>;
+    if constexpr (is_integral(c_mag))
-    if constexpr (std::is_floating_point_v<typename traits::multiplier_type>) {
+      return scale([&](auto value) { return value * get_value<multiplier_type>(numerator(c_mag)); });
-      // this results in great assembly
+    else if constexpr (is_integral(pow<-1>(c_mag)))
-      auto res = static_cast<MP_UNITS_TYPENAME To::rep>(
+      return scale([&](auto value) { return value / get_value<multiplier_type>(denominator(c_mag)); });
-        static_cast<traits::c_type>(q.numerical_value_is_an_implementation_detail_) * traits::ratio);
+    else {
-      return {res, To::reference};
+      using value_traits = conversion_value_traits<c_mag, multiplier_type>;
-    } else {
+      if constexpr (std::is_floating_point_v<multiplier_type>)
-      // this is slower but allows conversions like 2000 m -> 2 km without loosing data
+        // this results in great assembly
-      auto res = static_cast<MP_UNITS_TYPENAME To::rep>(
+        return scale([](auto value) { return value * value_traits::ratio; });
-        static_cast<traits::c_type>(q.numerical_value_is_an_implementation_detail_) * traits::num_mult /
+      else
-        traits::den_mult * traits::irr_mult);
+        // this is slower but allows conversions like 2000 m -> 2 km without loosing data
-      return {res, To::reference};
+        return scale(
+          [](auto value) { return value * value_traits::num_mult / value_traits::den_mult * value_traits::irr_mult; });
     }
   }
 }
@@ -126,21 +141,18 @@ template<Quantity To, typename From>
  *
  * @tparam ToQP a target quantity point type to which to cast to
  */
-template<QuantityPoint ToQP, typename FromQP>
+template<QuantityPoint ToQP, typename FwdFromQP, typename FromQP = std::remove_cvref_t<FwdFromQP>>
-  requires QuantityPoint<std::remove_cvref_t<FromQP>> &&
+  requires QuantityPoint<FromQP> && (castable(FromQP::quantity_spec, ToQP::quantity_spec)) &&
-           (castable(std::remove_reference_t<FromQP>::quantity_spec, ToQP::quantity_spec)) &&
+           (detail::same_absolute_point_origins(ToQP::point_origin, FromQP::point_origin)) &&
-           (detail::same_absolute_point_origins(ToQP::point_origin, std::remove_reference_t<FromQP>::point_origin)) &&
+           ((FromQP::unit == ToQP::unit && std::constructible_from<typename ToQP::rep, typename FromQP::rep>) ||
-           ((std::remove_reference_t<FromQP>::unit == ToQP::unit &&
+            (FromQP::unit != ToQP::unit))
-             std::constructible_from<typename ToQP::rep, typename std::remove_reference_t<FromQP>::rep>) ||
+[[nodiscard]] constexpr QuantityPoint auto sudo_cast(FwdFromQP&& qp)
-            (std::remove_reference_t<FromQP>::unit != ToQP::unit))
-[[nodiscard]] constexpr QuantityPoint auto sudo_cast(FromQP&& qp)
 {
-  using qp_type = std::remove_reference_t<FromQP>;
   if constexpr (is_same_v<std::remove_const_t<decltype(ToQP::point_origin)>,
-                          std::remove_const_t<decltype(qp_type::point_origin)>>) {
+                          std::remove_const_t<decltype(FromQP::point_origin)>>) {
     return quantity_point{
-      sudo_cast<typename ToQP::quantity_type>(std::forward<FromQP>(qp).quantity_from(qp_type::point_origin)),
+      sudo_cast<typename ToQP::quantity_type>(std::forward<FromQP>(qp).quantity_from(FromQP::point_origin)),
-      qp_type::point_origin};
+      FromQP::point_origin};
   } else {
     // it's unclear how hard we should try to avoid truncation here. For now, the only corner case we cater for,
     // is when the range of the quantity type of at most one of QP or ToQP doesn't cover the offset between the
@@ -152,23 +164,26 @@ template<QuantityPoint ToQP, typename FromQP>
     //  (c) add/subtract the origin difference
     // In the following, we carefully select the order of these three operations: each of (a) and (b) is scheduled
     // either before or after (c), such that (c) acts on the largest range possible among all combination of source
-    // and target unit and represenation.
+    // and target unit and representation.
-    using traits = magnitude_conversion_traits<typename ToQP::quantity_type, typename qp_type::quantity_type>;
+    static constexpr Magnitude auto c_mag = get_canonical_unit(FromQP::unit).mag / get_canonical_unit(ToQP::unit).mag;
-    using c_rep_type = typename traits::c_rep_type;
+    using type_traits = conversion_type_traits<c_mag, typename FromQP::rep, typename ToQP::rep>;
-    if constexpr (traits::num_mult * traits::irr_mult > traits::den_mult) {
+    using value_traits = conversion_value_traits<c_mag, typename type_traits::multiplier_type>;
+    using c_rep_type = typename type_traits::c_rep_type;
+    if constexpr (value_traits::num_mult * value_traits::irr_mult > value_traits::den_mult) {
       // original unit had a larger unit magnitude; if we first convert to the common representation but retain the
       // unit, we obtain the largest possible range while not causing truncation of fractional values. This is optimal
       // for the offset computation.
       return sudo_cast<ToQP>(
-        sudo_cast<quantity_point<qp_type::reference, qp_type::point_origin, c_rep_type>>(std::forward<FromQP>(qp))
+        sudo_cast<quantity_point<FromQP::reference, FromQP::point_origin, c_rep_type>>(std::forward<FromQP>(qp))
           .point_for(ToQP::point_origin));
     } else {
       // new unit may have a larger unit magnitude; we first need to convert to the new unit (potentially causing
       // truncation, but no more than if we did the conversion later), but make sure we keep the larger of the two
       // representation types. Then, we can perform the offset computation.
-      return sudo_cast<ToQP>(sudo_cast<quantity_point<make_reference(qp_type::quantity_spec, ToQP::unit),
+      return sudo_cast<ToQP>(
-                                                      qp_type::point_origin, c_rep_type>>(std::forward<FromQP>(qp))
+        sudo_cast<quantity_point<make_reference(FromQP::quantity_spec, ToQP::unit), FromQP::point_origin, c_rep_type>>(
-                               .point_for(ToQP::point_origin));
+          std::forward<FromQP>(qp))
+          .point_for(ToQP::point_origin));
     }
   }
 }

test/static/quantity_test.cpp

@@ -260,6 +260,9 @@ static_assert(quantity<isq::length[km], int>(2 * km).force_in(km).numerical_valu
 static_assert(quantity<isq::length[km], int>(2 * km).force_in(m).numerical_value_in(m) == 2000);
 static_assert(quantity<isq::length[m], int>(2000 * m).force_in(km).numerical_value_in(km) == 2);
 
+static_assert((15. * m).in(nm).numerical_value_in(m) == 15.);
+static_assert((15'000. * nm).in(m).numerical_value_in(nm) == 15'000.);
+
 template<template<auto, typename> typename Q>
 concept invalid_unit_conversion = requires {
   requires !requires { Q<isq::length[m], int>(2000 * m).in(km); };     // truncating conversion