From bb7911f10880b6a1899b7acbe6bc29daa8d0a6c5 Mon Sep 17 00:00:00 2001 From: Chip Hogg Date: Mon, 2 Dec 2024 14:51:58 -0500 Subject: [PATCH] Convert `CommonPointUnit` to EQUIV-style labels (#339) This is a kind of "good enough for now" placeholder. It's a big improvement over what we had before, because it shows the actual values. There may be some subtleties that we could improve on, but we will save them for a future release. Fixes #105. --- au/code/au/quantity_point_test.cc | 23 ++++++++++++++++++++++- au/code/au/unit_of_measure.hh | 14 +++++--------- au/code/au/unit_of_measure_test.cc | 30 ++++++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/au/code/au/quantity_point_test.cc b/au/code/au/quantity_point_test.cc index 9f911956..034ae556 100644 --- a/au/code/au/quantity_point_test.cc +++ b/au/code/au/quantity_point_test.cc @@ -21,8 +21,17 @@ using ::testing::Not; using ::testing::StaticAssertTypeEq; +using ::testing::StrEq; namespace au { +namespace { +template +std::string stream_to_string(const T &t) { + std::ostringstream oss; + oss << t; + return oss.str(); +} +} // namespace struct Meters : UnitImpl {}; constexpr QuantityMaker meters{}; @@ -34,7 +43,10 @@ constexpr auto inches_pt = QuantityPointMaker{}; struct Feet : decltype(Inches{} * mag<12>()) {}; constexpr auto feet_pt = QuantityPointMaker{}; -struct Kelvins : UnitImpl {}; +struct Kelvins : UnitImpl { + static constexpr const char label[] = "K"; +}; +constexpr const char Kelvins::label[]; constexpr QuantityMaker kelvins{}; constexpr QuantityPointMaker kelvins_pt{}; @@ -44,7 +56,10 @@ struct Celsius : Kelvins { // little as possible (while still keeping the value an integer), because that will make the // conversion factor as small as possible in converting to the common-point-unit. static constexpr auto origin() { return (kelvins / mag<20>())(273'15 / 5); } + + static constexpr const char label[] = "degC"; }; +constexpr const char Celsius::label[]; constexpr QuantityMaker celsius_qty{}; constexpr QuantityPointMaker celsius_pt{}; @@ -345,6 +360,12 @@ TEST(QuantityPoint, MixedUnitAndRepDifferenceUsesCommonPointType) { EXPECT_THAT(rep_wins - unit_wins, QuantityEquivalent(meters(100.0))); } +TEST(QuantityPoint, CommonPointUnitLabel) { + EXPECT_THAT(stream_to_string(celsius_pt(0) - kelvins_pt(0)), + AnyOf(StrEq("5463 EQUIV{[(1 / 20) K], [(1 / 20) degC]}"), + StrEq("5463 EQUIV{[(1 / 20) degC], [(1 / 20) K]}"))); +} + TEST(QuantityPoint, CanCompareUnitsWithDifferentOrigins) { EXPECT_THAT(celsius_pt(0), ConsistentlyGreaterThan(kelvins_pt(273))); EXPECT_THAT(celsius_pt(0), ConsistentlyEqualTo(milli(kelvins_pt)(273'150))); diff --git a/au/code/au/unit_of_measure.hh b/au/code/au/unit_of_measure.hh index 0f8580c2..538a2bab 100644 --- a/au/code/au/unit_of_measure.hh +++ b/au/code/au/unit_of_measure.hh @@ -912,16 +912,12 @@ struct UnitLabel> : CommonUnitLabel>{} / detail::MagT{}))...> {}; -// Implementation for CommonPointUnit: unite constituent labels. +// Implementation for CommonPointUnit: give size in terms of each constituent unit, taking any +// origin displacements into account. template -struct UnitLabel> { - using LabelT = detail::ExtendedLabel<8 + 2 * (sizeof...(Us) - 1), Us...>; - static constexpr LabelT value = - detail::concatenate("COM_PT[", detail::join_by(", ", unit_label(Us{})...), "]"); -}; -template -constexpr - typename UnitLabel>::LabelT UnitLabel>::value; +struct UnitLabel> + : CommonUnitLabel>{} / detail::MagT{}))...> {}; template constexpr const auto &unit_label(Unit) { diff --git a/au/code/au/unit_of_measure_test.cc b/au/code/au/unit_of_measure_test.cc index 83e151f3..8b3b6b9a 100644 --- a/au/code/au/unit_of_measure_test.cc +++ b/au/code/au/unit_of_measure_test.cc @@ -45,7 +45,9 @@ constexpr auto common_unit(Us...) { struct Celsius : Kelvins { static constexpr auto origin() { return milli(kelvins)(273'150); } + static constexpr const char label[] = "deg_C"; }; +constexpr const char Celsius::label[]; constexpr auto celsius = QuantityMaker{}; struct AlternateCelsius : Kelvins { @@ -437,7 +439,9 @@ TEST(OriginDisplacement, FunctionalInterfaceHandlesInstancesCorrectly) { struct OffsetCelsius : Celsius { static constexpr auto origin() { return detail::OriginOf::value() + kelvins(10); } + static constexpr const char label[] = "offset_deg_C"; }; +constexpr const char OffsetCelsius::label[]; TEST(DisplaceOrigin, DisplacesOrigin) { EXPECT_EQ(origin_displacement(Celsius{}, OffsetCelsius{}), kelvins(10)); @@ -462,6 +466,11 @@ TEST(CommonUnit, PrefersUnitFromListIfAnyIdentical) { StaticAssertTypeEq, Inches>(); } +TEST(CommonUnit, DedupesUnitsMadeIdenticalAfterUnscalingSameScaledUnit) { + StaticAssertTypeEq()), decltype(Feet{} * mag<5>())>, + Feet>(); +} + TEST(CommonUnit, DownranksAnonymousScaledUnits) { StaticAssertTypeEq())>, Yards>(); } @@ -648,13 +657,30 @@ TEST(UnitLabel, ReducesToSingleUnitLabelIfAllUnitsAreTheSame) { TEST(UnitLabel, LabelsCommonPointUnitCorrectly) { using U = CommonPointUnitT; - EXPECT_THAT(unit_label(U{}), AnyOf(StrEq("COM_PT[in, m]"), StrEq("COM_PT[m, in]"))); + EXPECT_THAT(unit_label(U{}), + AnyOf(StrEq("EQUIV{[(1 / 127) in], [(1 / 5000) m]}"), + StrEq("EQUIV{[(1 / 5000) m], [(1 / 127) in]}"))); } TEST(UnitLabel, CommonPointUnitLabelWorksWithUnitProduct) { using U = CommonPointUnitT, UnitQuotientT>; EXPECT_THAT(unit_label(U{}), - AnyOf(StrEq("COM_PT[m / min, in / min]"), StrEq("COM_PT[in / min, m / min]"))); + AnyOf(StrEq("EQUIV{[(1 / 127) in / min], [(1 / 5000) m / min]}"), + StrEq("EQUIV{[(1 / 5000) m / min], [(1 / 127) in / min]}"))); +} + +TEST(UnitLabel, CommonPointUnitLabelTakesOriginOffsetIntoAccount) { + // NOTE: the (1 / 1000) scaling comes about because we're still using `Quantity` to define our + // origins. We may be able to do better, and re-found them on `Constant`, at least once we add + // some kind of subtraction that works at least some of the time. The ideal end point would be + // that the common point unit for `Celsius` and `OffsetCelsius` would be simply `Celsius` + // (because `OffsetCelsius` is just `Celsius` with a _higher_ origin). At that point, we'll + // need to construct a more complicated example here that will force us to use + // `CommonPointUnit<...>`, because it will be meaningfully different from all of its parameters. + using U = CommonPointUnitT; + EXPECT_THAT(unit_label(U{}), + AnyOf(StrEq("EQUIV{[(1 / 1000) deg_C], [(1 / 1000) offset_deg_C]}"), + StrEq("EQUIV{[(1 / 1000) offset_deg_C], [(1 / 1000) deg_C]}"))); } TEST(UnitLabel, APICompatibleWithUnitSlots) { EXPECT_THAT(unit_label(feet), StrEq("ft")); }