Refine and document conversion factor policies

au/BUILD.bazel

@@ -105,6 +105,24 @@ cc_test(
 ################################################################################
 # Implementation detail libraries and tests
 
+cc_library(
+    name = "apply_magnitude",
+    hdrs = ["apply_magnitude.hh"],
+    deps = [":magnitude"],
+)
+
+cc_test(
+    name = "apply_magnitude_test",
+    size = "small",
+    srcs = ["apply_magnitude_test.cc"],
+    copts = ["-Iexternal/gtest/include"],
+    deps = [
+        ":apply_magnitude",
+        ":testing",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_library(
     name = "chrono_policy_validation",
     testonly = True,
@@ -304,6 +322,7 @@ cc_library(
     name = "quantity",
     hdrs = ["quantity.hh"],
     deps = [
+        ":apply_magnitude",
         ":conversion_policy",
         ":operators",
         ":unit_of_measure",

au/apply_magnitude.hh

@@ -0,0 +1,115 @@
+// Copyright 2023 Aurora Operations, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#pragma once
+
+#include "au/magnitude.hh"
+
+namespace au {
+namespace detail {
+
+// The various categories by which a magnitude can be applied to a numeric quantity.
+enum class ApplyAs {
+    INTEGER_MULTIPLY,
+    INTEGER_DIVIDE,
+    RATIONAL_MULTIPLY,
+    IRRATIONAL_MULTIPLY,
+};
+
+template <typename... BPs>
+constexpr ApplyAs categorize_magnitude(Magnitude<BPs...>) {
+    if (IsInteger<Magnitude<BPs...>>::value) {
+        return ApplyAs::INTEGER_MULTIPLY;
+    }
+
+    if (IsInteger<MagInverseT<Magnitude<BPs...>>>::value) {
+        return ApplyAs::INTEGER_DIVIDE;
+    }
+
+    return IsRational<Magnitude<BPs...>>::value ? ApplyAs::RATIONAL_MULTIPLY
+                                                : ApplyAs::IRRATIONAL_MULTIPLY;
+}
+
+template <typename Mag, ApplyAs Category, typename T, bool is_T_integral>
+struct ApplyMagnitudeImpl;
+
+// Multiplying by an integer, for any type T.
+template <typename Mag, typename T, bool is_T_integral>
+struct ApplyMagnitudeImpl<Mag, ApplyAs::INTEGER_MULTIPLY, T, is_T_integral> {
+    static_assert(categorize_magnitude(Mag{}) == ApplyAs::INTEGER_MULTIPLY,
+                  "Mismatched instantiation (should never be done manually)");
+    static_assert(is_T_integral == std::is_integral<T>::value,
+                  "Mismatched instantiation (should never be done manually)");
+
+    constexpr T operator()(const T &x) { return x * get_value<T>(Mag{}); }
+};
+
+// Dividing by an integer, for any type T.
+template <typename Mag, typename T, bool is_T_integral>
+struct ApplyMagnitudeImpl<Mag, ApplyAs::INTEGER_DIVIDE, T, is_T_integral> {
+    static_assert(categorize_magnitude(Mag{}) == ApplyAs::INTEGER_DIVIDE,
+                  "Mismatched instantiation (should never be done manually)");
+    static_assert(is_T_integral == std::is_integral<T>::value,
+                  "Mismatched instantiation (should never be done manually)");
+
+    constexpr T operator()(const T &x) { return x / get_value<T>(MagInverseT<Mag>{}); }
+};
+
+// Applying a (non-integer, non-inverse-integer) rational, for any integral type T.
+template <typename Mag, typename T>
+struct ApplyMagnitudeImpl<Mag, ApplyAs::RATIONAL_MULTIPLY, T, true> {
+    static_assert(categorize_magnitude(Mag{}) == ApplyAs::RATIONAL_MULTIPLY,
+                  "Mismatched instantiation (should never be done manually)");
+    static_assert(std::is_integral<T>::value,
+                  "Mismatched instantiation (should never be done manually)");
+
+    constexpr T operator()(const T &x) {
+        return x * get_value<T>(numerator(Mag{})) / get_value<T>(denominator(Mag{}));
+    }
+};
+
+// Applying a (non-integer, non-inverse-integer) rational, for any non-integral type T.
+template <typename Mag, typename T>
+struct ApplyMagnitudeImpl<Mag, ApplyAs::RATIONAL_MULTIPLY, T, false> {
+    static_assert(categorize_magnitude(Mag{}) == ApplyAs::RATIONAL_MULTIPLY,
+                  "Mismatched instantiation (should never be done manually)");
+    static_assert(!std::is_integral<T>::value,
+                  "Mismatched instantiation (should never be done manually)");
+
+    constexpr T operator()(const T &x) { return x * get_value<T>(Mag{}); }
+};
+
+// Applying an irrational for any type T (although only non-integral T makes sense).
+template <typename Mag, typename T, bool is_T_integral>
+struct ApplyMagnitudeImpl<Mag, ApplyAs::IRRATIONAL_MULTIPLY, T, is_T_integral> {
+    static_assert(!std::is_integral<T>::value, "Cannot apply irrational magnitude to integer type");
+
+    static_assert(categorize_magnitude(Mag{}) == ApplyAs::IRRATIONAL_MULTIPLY,
+                  "Mismatched instantiation (should never be done manually)");
+    static_assert(is_T_integral == std::is_integral<T>::value,
+                  "Mismatched instantiation (should never be done manually)");
+
+    constexpr T operator()(const T &x) { return x * get_value<T>(Mag{}); }
+};
+
+template <typename T, typename... BPs>
+constexpr T apply_magnitude(const T &x, Magnitude<BPs...> m) {
+    return ApplyMagnitudeImpl<Magnitude<BPs...>,
+                              categorize_magnitude(m),
+                              T,
+                              std::is_integral<T>::value>{}(x);
+}
+
+}  // namespace detail
+}  // namespace au

au/apply_magnitude_test.cc

@@ -0,0 +1,132 @@
+// Copyright 2023 Aurora Operations, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "au/apply_magnitude.hh"
+
+#include "au/testing.hh"
+#include "gtest/gtest.h"
+
+using ::testing::ElementsAreArray;
+using ::testing::Not;
+
+namespace au {
+namespace detail {
+namespace {
+template <typename T>
+std::vector<T> first_n_positive_values(std::size_t n) {
+    std::vector<T> result;
+    result.reserve(n);
+    for (auto i = 1u; i <= n; ++i) {
+        result.push_back(static_cast<T>(i));
+    }
+    return result;
+}
+}  // namespace
+
+TEST(CategorizeMagnitude, FindsIntegerMultiplyInstances) {
+    EXPECT_EQ(categorize_magnitude(mag<2>()), ApplyAs::INTEGER_MULTIPLY);
+    EXPECT_EQ(categorize_magnitude(mag<35>()), ApplyAs::INTEGER_MULTIPLY);
+
+    EXPECT_EQ(categorize_magnitude(mag<35>() / mag<7>()), ApplyAs::INTEGER_MULTIPLY);
+}
+
+TEST(CategorizeMagnitude, FindsIntegerDivideInstances) {
+    EXPECT_EQ(categorize_magnitude(ONE / mag<2>()), ApplyAs::INTEGER_DIVIDE);
+    EXPECT_EQ(categorize_magnitude(ONE / mag<35>()), ApplyAs::INTEGER_DIVIDE);
+
+    EXPECT_EQ(categorize_magnitude(mag<7>() / mag<35>()), ApplyAs::INTEGER_DIVIDE);
+}
+
+TEST(CategorizeMagnitude, FindsRationalMultiplyInstances) {
+    EXPECT_EQ(categorize_magnitude(mag<5>() / mag<2>()), ApplyAs::RATIONAL_MULTIPLY);
+}
+
+TEST(CategorizeMagnitude, FindsIrrationalMultiplyInstances) {
+    EXPECT_EQ(categorize_magnitude(sqrt(mag<2>())), ApplyAs::IRRATIONAL_MULTIPLY);
+    EXPECT_EQ(categorize_magnitude(PI), ApplyAs::IRRATIONAL_MULTIPLY);
+}
+
+TEST(ApplyMagnitude, MultipliesForIntegerMultiply) {
+    constexpr auto m = mag<25>();
+    ASSERT_EQ(categorize_magnitude(m), ApplyAs::INTEGER_MULTIPLY);
+
+    EXPECT_THAT(apply_magnitude(4, m), SameTypeAndValue(100));
+    EXPECT_THAT(apply_magnitude(4.0f, m), SameTypeAndValue(100.0f));
+}
+
+TEST(ApplyMagnitude, DividesForIntegerDivide) {
+    constexpr auto one_thirteenth = ONE / mag<13>();
+    ASSERT_EQ(categorize_magnitude(one_thirteenth), ApplyAs::INTEGER_DIVIDE);
+
+    // This test would fail if our implementation multiplied by the float representation of (1/13),
+    // instead of dividing by 13, under the hood.
+    for (const auto &i : first_n_positive_values<float>(100u)) {
+        EXPECT_THAT(apply_magnitude(i * 13, one_thirteenth), SameTypeAndValue(i));
+    }
+}
+
+TEST(ApplyMagnitude, MultipliesThenDividesForRationalMagnitudeOnInteger) {
+    // Consider applying the magnitude (3/2) to the value 5.  The exact answer is the real number
+    // 7.5, which becomes 7 when translated (via truncation) to the integer domain.
+    //
+    // If we multiply-then-divide, we get (5 * 3) / 2 = 7, which is correct.
+    //
+    // If we divide-then-multiply --- say, because we are trying to avoid overflow --- then we get
+    // (5 / 2) * 3 = 2 * 3 = 6, which is wrong.
+    constexpr auto three_halves = mag<3>() / mag<2>();
+    ASSERT_EQ(categorize_magnitude(three_halves), ApplyAs::RATIONAL_MULTIPLY);
+
+    EXPECT_THAT(apply_magnitude(5, three_halves), SameTypeAndValue(7));
+}
+
+TEST(ApplyMagnitude, MultipliesSingleNumberForRationalMagnitudeOnFloatingPoint) {
+    // Helper similar to `std::transform`, but with more convenient interfaces.
+    auto apply = [](std::vector<float> vals, auto fun) {
+        for (auto &v : vals) {
+            v = fun(v);
+        }
+        return vals;
+    };
+
+    // Create our rational magnitude, (2 / 13).
+    constexpr auto two_thirteenths = mag<2>() / mag<13>();
+    ASSERT_EQ(categorize_magnitude(two_thirteenths), ApplyAs::RATIONAL_MULTIPLY);
+
+    // Test a bunch of values.  We are hoping that the two different strategies will yield different
+    // results for at least some of these strategies (and we'll check that this is the case).
+    const auto original_vals = first_n_positive_values<float>(10u);
+
+    // Compute expected answers for each possible strategy.
+    const auto if_we_multiply_and_divide =
+        apply(original_vals, [](float v) { return v * 2.0f / 13.0f; });
+    const auto if_we_use_one_factor =
+        apply(original_vals, [](float v) { return v * (2.0f / 13.0f); });
+
+    // The strategies must be different for at least some results!
+    ASSERT_THAT(if_we_multiply_and_divide, Not(ElementsAreArray(if_we_use_one_factor)));
+
+    // Make sure we follow the single-number strategy, every time.
+    const auto results =
+        apply(original_vals, [=](float v) { return apply_magnitude(v, two_thirteenths); });
+    EXPECT_THAT(results, ElementsAreArray(if_we_use_one_factor));
+    EXPECT_THAT(results, Not(ElementsAreArray(if_we_multiply_and_divide)));
+}
+
+TEST(ApplyMagnitude, MultipliesSingleNumberForIrrationalMagnitudeOnFloatingPoint) {
+    ASSERT_EQ(categorize_magnitude(PI), ApplyAs::IRRATIONAL_MULTIPLY);
+    EXPECT_THAT(apply_magnitude(2.0f, PI), SameTypeAndValue(2.0f * static_cast<float>(M_PI)));
+}
+
+}  // namespace detail
+}  // namespace au

au/quantity.hh

@@ -16,6 +16,7 @@
 
 #include <utility>
 
+#include "au/apply_magnitude.hh"
 #include "au/conversion_policy.hh"
 #include "au/operators.hh"
 #include "au/stdx/functional.hh"
@@ -143,17 +144,11 @@ class Quantity {
               typename NewUnit,
               typename = std::enable_if_t<IsUnit<NewUnit>::value>>
     constexpr auto as(NewUnit) const {
-        constexpr auto ratio = unit_ratio(unit, NewUnit{});
-
         using Common = std::common_type_t<Rep, NewRep>;
-        constexpr auto NUM = integer_part(numerator(ratio));
-        constexpr auto DEN = integer_part(denominator(ratio));
-        constexpr auto num = get_value<Common>(NUM);
-        constexpr auto den = get_value<Common>(DEN);
-        constexpr auto irr = get_value<Common>(ratio * DEN / NUM);
+        using Factor = UnitRatioT<Unit, NewUnit>;
 
         return make_quantity<NewUnit>(
-            static_cast<NewRep>(static_cast<Common>(value_) * num / den * irr));
+            static_cast<NewRep>(detail::apply_magnitude(static_cast<Common>(value_), Factor{})));
     }
 
     template <typename NewUnit, typename = std::enable_if_t<IsUnit<NewUnit>::value>>

au/quantity_test.cc

@@ -94,6 +94,14 @@ TEST(MakeQuantity, MakesQuantityInGivenUnit) {
     EXPECT_EQ(make_quantity<Feet>(99), feet(99));
 }
 
+TEST(Quantity, RationalConversionRecoversExactIntegerValues) {
+    // This test would fail if our implementation multiplied by the float
+    // representation of (1/13), instead of dividing by 13, under the hood.
+    for (int i = 1; i < 100; ++i) {
+        EXPECT_EQ(feet(static_cast<float>(i * 13)).in(feet * mag<13>()), i);
+    }
+}
+
 TEST(QuantityMaker, CreatesAppropriateQuantityIfCalled) { EXPECT_EQ(yards(3.14).in(yards), 3.14); }
 
 TEST(QuantityMaker, CanBeMultipliedBySingularUnitToGetMakerOfProductUnit) {