From e129b2228927af9c8d347c39c345ddd8cae20bb8 Mon Sep 17 00:00:00 2001 From: Yves Delley Date: Sat, 20 Jul 2024 12:30:08 +0200 Subject: [PATCH] change internal strategy to alwasy use the output unit --- src/core/include/mp-units/bits/sudo_cast.h | 60 ++++++++++++------- .../mp-units/framework/quantity_point.h | 7 ++- test/static/quantity_point_test.cpp | 17 ++++-- 3 files changed, 59 insertions(+), 25 deletions(-) diff --git a/src/core/include/mp-units/bits/sudo_cast.h b/src/core/include/mp-units/bits/sudo_cast.h index fd7a99739..3b47b70ba 100644 --- a/src/core/include/mp-units/bits/sudo_cast.h +++ b/src/core/include/mp-units/bits/sudo_cast.h @@ -136,11 +136,27 @@ template [[nodiscard]] constexpr QuantityPoint auto sudo_cast(FromQP&& qp) { using qp_type = std::remove_reference_t; - if constexpr (is_same_v, - std::remove_const_t>) { + using traits = magnitude_conversion_traits; + if constexpr (ToQP::point_origin == qp_type::point_origin) { + // no change of offset needed return quantity_point{ sudo_cast(std::forward(qp).quantity_from(qp_type::point_origin)), - qp_type::point_origin}; + ToQP::point_origin}; + } else if constexpr (qp_type::quantity_type::unit == ToQP::quantity_type::unit) { + // no scaling of the unit is needed; thus we can perform all computation in a single unit without any runtime + // scaling anywhere + using offset_rep_type = typename traits::c_rep_type; + // in the following, we take the detour through `quantity_point` to determine the offset between the two + // point_origins; there seem to be cases where we have two `zeroeth_point_origins` of different units (i.e. m vs. + // km), where the subtraction for the latter is not defined. + static constexpr auto zero = offset_rep_type{0} * qp_type::reference; + // we statically convert the offset to unit of the quantities, to avoid runtime rescaling. + // TODO: should we implement do a rounding instead here? + static constexpr auto offset = sudo_cast>( + quantity_point{zero, qp_type::point_origin} - quantity_point{zero, ToQP::point_origin}); + return quantity_point{ + sudo_cast(std::forward(qp).quantity_from(qp_type::point_origin) + offset), + ToQP::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 @@ -153,23 +169,27 @@ template // 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. - using traits = magnitude_conversion_traits; - using c_rep_type = typename traits::c_rep_type; - if constexpr (traits::num_mult * traits::irr_mult > 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( - sudo_cast>(std::forward(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(sudo_cast>(std::forward(qp)) - .point_for(ToQP::point_origin)); - } + // + // This implementation handles operations (a) and (b) by delegating to the `sudo_cast`, + // while opertaion (c) is delegated to quantity::operator+. We ensure that no hidden conversions happen in the + // latter, by proving both operands in the same representation, the `intermediate_quantity_type`. The + // `intermediate_quantity_type` in turn is chosen such that operations (a) and (b) individually happen either during + // the inital conversion from the input, or during the final conversion to the output, whichever is optimal given + // the input and output types. + static constexpr auto intermediate_reference = make_reference(qp_type::quantity_spec, ToQP::unit); + // We always work in the larger of the two (input/output) representations. + using intermediate_rep_type = typename traits::c_type; + using intermediate_quantity_type = quantity; + // in the following, we take the detour through `quantity_point` to determine the offset between the two + // point_origins; there seem to be cases where we have two `zeroeth_point_origins` of different units (i.e. m vs. + // km), where the subtraction for the latter is not defined. + static constexpr auto zero = intermediate_rep_type{0} * intermediate_reference; + static constexpr intermediate_quantity_type offset = + quantity_point{zero, qp_type::point_origin} - quantity_point{zero, ToQP::point_origin}; + return quantity_point{ + sudo_cast( + sudo_cast(std::forward(qp).quantity_from(qp_type::point_origin)) + offset), + ToQP::point_origin}; } } diff --git a/src/core/include/mp-units/framework/quantity_point.h b/src/core/include/mp-units/framework/quantity_point.h index 29e29a2f7..1985f29a6 100644 --- a/src/core/include/mp-units/framework/quantity_point.h +++ b/src/core/include/mp-units/framework/quantity_point.h @@ -238,7 +238,12 @@ class quantity_point { requires std::constructible_from // NOLINTNEXTLINE(google-explicit-constructor, hicpp-explicit-conversions) constexpr explicit(!std::convertible_to) quantity_point(const QP& qp) : - quantity_point(detail::sudo_cast(qp)) + quantity_from_origin_is_an_implementation_detail_([&] { + if constexpr (point_origin == QP::point_origin) + return qp.quantity_ref_from(point_origin); + else + return qp - point_origin; + }()) { } diff --git a/test/static/quantity_point_test.cpp b/test/static/quantity_point_test.cpp index 06ad1f16a..39a689ec9 100644 --- a/test/static/quantity_point_test.cpp +++ b/test/static/quantity_point_test.cpp @@ -1747,13 +1747,22 @@ static_assert(value_cast>(quantity_point{2 * isq: static_assert(value_cast>(quantity_point{2000 * isq::height[m]}) .quantity_from_origin_is_an_implementation_detail_.numerical_value_in(km) == 2); // a value_cast which includes a change to the point origin -static_assert(value_cast>(quantity_point{2000 * isq::height[m], - ground_level}) +static_assert(value_cast>(quantity_point{int{2000} * isq::height[m], + ground_level}) + .quantity_from_origin_is_an_implementation_detail_.numerical_value_in(m) == 2042); +// a value_cast which includes a change to the point origin and the representation +static_assert(value_cast>(quantity_point{2000 * isq::height[m], + ground_level}) .quantity_from_origin_is_an_implementation_detail_.numerical_value_in(m) == 2042); // a value_cast which includes a change to the point origin as-well as a change in units -static_assert(value_cast>(quantity_point{2 * isq::height[km], - ground_level}) +static_assert(value_cast>(quantity_point{int{2} * isq::height[km], + ground_level}) + .quantity_from_origin_is_an_implementation_detail_.numerical_value_in(m) == 2042); +// a value_cast which includes a change to the point origin as-well as a change in units and the representation +static_assert(value_cast>(quantity_point{2 * isq::height[km], + ground_level}) .quantity_from_origin_is_an_implementation_detail_.numerical_value_in(m) == 2042); + // a value_cast which changes all three of unit, rep, point_origin simultaneously, and the range of either FromQP or // ToQP does not include the other's point_origin static_assert(value_cast>(