From a6b6afe438030ffe02bfe326e6c2942020f1a755 Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Fri, 28 Jan 2022 19:06:26 +0000
Subject: [PATCH 01/16] Add value and categorization helpers for Magnitude

For each Magnitude `m`, we now support:

- `is_rational(m)` tells whether it represents a rational number.
- `is_integral(m)` tells whether it represents an integer (and
  `is_integral(m)` implies `is_rational(m)`).
- `m.value<T>` represents the value of `m` in the type `T`.

If `T` is integral, we only support `m.value<T>` when `is_integral(m)`.

We also perform all intermediate computations in the widest type of the
same category (floating point, signed, or unsigned).  This means we can,
for example, give first-class support to embedded users who may have
hardware support only for `float`: we can ensure they get the most
accurate `float` value we can compute, without burdening them with a
dependency on `long double` in their runtime code.

We are not yet ready to replace `ratio` as the definition of unit
magnitudes, but we're a step closer.  The next step will be to decompose
a Magnitude into numerator, denominator, and irrational parts, giving us
full bidirectional convertibility with existing `ratio` instances.  Then
we can migrate over in a controlled fashion.
---
 src/core/include/units/magnitude.h        | 115 +++++++++++++++++++++-
 test/unit_test/runtime/magnitude_test.cpp | 111 +++++++++++++++++++++
 2 files changed, 225 insertions(+), 1 deletion(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index 8e9c46ca..c33a61d3 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -25,6 +25,7 @@
 #include <units/ratio.h>
 #include <cstdint>
 #include <numbers>
+#include <stdexcept>
 
 namespace units {
 
@@ -117,6 +118,98 @@ static constexpr bool is_base_power<base_power<T>> = true;
 template<typename T>
 concept BasePower = detail::is_base_power<T>;
 
+namespace detail
+{
+constexpr auto inverse(BasePower auto bp) {
+  bp.power.num *= -1;
+  return bp;
+}
+
+// `widen_t` gives the widest arithmetic type in the same category, for intermediate computations.
+template<typename T> requires std::is_arithmetic_v<T>
+using widen_t = std::conditional_t<
+  std::is_floating_point_v<T>,
+  long double,
+  std::conditional_t<std::is_signed_v<T>, std::intmax_t, std::uintmax_t>>;
+
+// Raise an arbitrary arithmetic type to a positive integer power at compile time.
+template<typename T> requires std::is_arithmetic_v<T>
+consteval T int_power(T base, std::integral auto exp){
+  if (exp < 0) { throw std::invalid_argument{"int_power only supports positive integer powers"}; }
+
+  // TODO(chogg): Unify this implementation with the one in pow.h.  That one takes its exponent as a
+  // template parameter, rather than a function parameter.
+
+  if (exp == 0) {
+    return 1;
+  }
+
+  if (exp % 2 == 1) {
+    return base * int_power(base, exp - 1);
+  }
+
+  const auto square_root = int_power(base, exp / 2);
+  const auto result = square_root * square_root;
+
+  if constexpr(std::is_unsigned_v<T>) {
+    // As this function can only be called at compile time, the exception functions as a
+    // "parameter-compatible static_assert", and does not result in exceptions at runtime.
+    if (result / square_root != square_root) { throw std::overflow_error{"Unsigned wraparound"}; }
+  }
+
+  return result;
+}
+
+
+template<typename T> requires std::is_arithmetic_v<T>
+consteval widen_t<T> compute_base_power(BasePower auto bp)
+{
+  // This utility can only handle integer powers.  To compute rational powers at compile time, we'll
+  // need to write a custom function.
+  //
+  // Note that since this function can only be called at compile time, the point of these exceptions
+  // is to act as "static_assert substitutes", not to throw actual exceptions at runtime.
+  if (bp.power.den != 1) { throw std::invalid_argument{"Rational powers not yet supported"}; }
+  if (bp.power.exp < 0) { throw std::invalid_argument{"Unsupported exp value"}; }
+
+  if (bp.power.num < 0) {
+    if constexpr (std::is_integral_v<T>) {
+      throw std::invalid_argument{"Cannot represent reciprocal as integer"};
+    } else {
+      return 1 / compute_base_power<T>(inverse(bp));
+    }
+  }
+
+  auto power = bp.power.num * int_power(10, bp.power.exp);
+  return int_power(static_cast<widen_t<T>>(bp.get_base()), power);
+}
+
+// A converter for the value member variable of magnitude (below).
+//
+// The input is the desired result, but in a (wider) intermediate type.  The point of this function
+// is to cast to the desired type, but avoid overflow in doing so.
+template<typename To, typename From>
+  requires std::is_arithmetic_v<To>
+    && std::is_arithmetic_v<From>
+    && (std::is_integral_v<To> == std::is_integral_v<From>)
+consteval To checked_static_cast(From x) {
+  // This function can only ever be called at compile time.  The purpose of these exceptions is to
+  // produce compiler errors, because we cannot `static_assert` on function arguments.
+  if constexpr (std::is_integral_v<To>) {
+    if (std::cmp_less(x, std::numeric_limits<To>::min()) ||
+        std::cmp_greater(x, std::numeric_limits<To>::max())) {
+      throw std::invalid_argument{"Cannot represent magnitude in this type"};
+    }
+  } else {
+    if (x < std::numeric_limits<To>::min() || x > std::numeric_limits<To>::max()) {
+      throw std::invalid_argument{"Cannot represent magnitude in this type"};
+    }
+  }
+
+  return static_cast<To>(x);
+}
+} // namespace detail
+
 /**
  * @brief  Equality detection for two base powers.
  */
@@ -215,6 +308,14 @@ static constexpr bool all_bases_in_order = strictly_increasing(BPs.get_base()...
 
 template<BasePower auto... BPs>
 static constexpr bool is_base_power_pack_valid = all_base_powers_valid<BPs...> && all_bases_in_order<BPs...>;
+
+constexpr bool is_rational(BasePower auto bp) {
+  return std::is_integral_v<decltype(bp.get_base())> && (bp.power.den == 1) && (bp.power.exp >= 0);
+}
+
+constexpr bool is_integral(BasePower auto bp) {
+  return is_rational(bp) && bp.power.num > 0;
+}
 } // namespace detail
 
 /**
@@ -225,7 +326,19 @@ static constexpr bool is_base_power_pack_valid = all_base_powers_valid<BPs...> &
  */
 template<BasePower auto... BPs>
   requires (detail::is_base_power_pack_valid<BPs...>)
-struct magnitude {};
+struct magnitude {
+  // Whether this magnitude represents an integer.
+  friend constexpr bool is_integral(magnitude) { return (detail::is_integral(BPs) && ...); }
+
+  // Whether this magnitude represents a rational number.
+  friend constexpr bool is_rational(magnitude) { return (detail::is_rational(BPs) && ...); }
+
+  // The value of this magnitude, expressed in a given type.
+  template<typename T>
+    requires (is_integral(magnitude{}) || std::is_floating_point_v<T>)
+  static constexpr T value = detail::checked_static_cast<T>(
+      (detail::compute_base_power<T>(BPs) * ...));
+};
 
 // Implementation for Magnitude concept (below).
 namespace detail {
diff --git a/test/unit_test/runtime/magnitude_test.cpp b/test/unit_test/runtime/magnitude_test.cpp
index 653a6ef8..f41e1955 100644
--- a/test/unit_test/runtime/magnitude_test.cpp
+++ b/test/unit_test/runtime/magnitude_test.cpp
@@ -37,6 +37,12 @@ struct invalid_negative_base { static constexpr long double value = -1.234L; };
 template<ratio Power>
 constexpr auto pi_to_the() { return magnitude<base_power<pi_base>{Power}>{}; }
 
+template<typename T, typename U>
+void check_same_type_and_value(T actual, U expected) {
+  CHECK(std::is_same_v<T, U>);
+  CHECK(actual == expected);
+}
+
 TEST_CASE("base_power")
 {
   SECTION("base rep deducible for integral base")
@@ -129,6 +135,69 @@ TEST_CASE("make_ratio performs prime factorization correctly")
   }
 }
 
+TEST_CASE("magnitude converts to numerical value")
+{
+  SECTION("Positive integer powers of integer bases give integer values")
+  {
+    constexpr auto mag_412 = as_magnitude<412>();
+    check_same_type_and_value(mag_412.value<int>, 412);
+    check_same_type_and_value(mag_412.value<std::size_t>, std::size_t{412});
+    check_same_type_and_value(mag_412.value<float>, 412.0f);
+    check_same_type_and_value(mag_412.value<double>, 412.0);
+  }
+
+  SECTION("Negative integer powers of integer bases compute correct values")
+  {
+    constexpr auto mag_0p125 = as_magnitude<ratio{1, 8}>();
+    check_same_type_and_value(mag_0p125.value<float>, 0.125f);
+    check_same_type_and_value(mag_0p125.value<double>, 0.125);
+  }
+
+  SECTION("pi to the 1 supplies correct values")
+  {
+    constexpr auto pi = pi_to_the<1>();
+    check_same_type_and_value(pi.value<float>, std::numbers::pi_v<float>);
+    check_same_type_and_value(pi.value<double>, std::numbers::pi_v<double>);
+    check_same_type_and_value(pi.value<long double>, std::numbers::pi_v<long double>);
+  }
+
+  SECTION("pi to arbitrary power performs computations in most accurate type at compile time")
+  {
+    constexpr auto pi_cubed = pi_to_the<3>();
+
+    auto cube = [](auto x) { return x * x * x; };
+    constexpr auto via_float = cube(std::numbers::pi_v<float>);
+    constexpr auto via_long_double = static_cast<float>(cube(std::numbers::pi_v<long double>));
+
+    constexpr auto pi_cubed_value = pi_cubed.value<float>;
+    REQUIRE(pi_cubed_value != via_float);
+    CHECK(pi_cubed_value == via_long_double);
+  }
+
+  SECTION("Impossible requests are prevented at compile time")
+  {
+    // Naturally, we cannot actually write a test to verify a compiler error.  But any of these can
+    // be uncommented if desired to verify that it breaks the build.
+
+    // (void)as_magnitude<412>().value<int8_t>;
+
+    // Would work for pow<62>:
+    // (void)pow<63>(as_magnitude<2>()).value<int64_t>;
+
+    // Would work for pow<63>:
+    // (void)pow<64>(as_magnitude<2>()).value<uint64_t>;
+
+    (void)pow<308>(as_magnitude<10>()).value<double>; // Compiles, correctly.
+    // (void)pow<309>(as_magnitude<10>()).value<double>;
+    // (void)pow<3099>(as_magnitude<10>()).value<double>;
+    // (void)pow<3099999>(as_magnitude<10>()).value<double>;
+
+    auto sqrt_2 = pow<ratio{1, 2}>(as_magnitude<2>());
+    CHECK(!is_integral(sqrt_2));
+    // (void)sqrt_2.value<int>;
+  }
+}
+
 TEST_CASE("Equality works for magnitudes")
 {
   SECTION("Equivalent ratios are equal")
@@ -218,8 +287,50 @@ TEST_CASE("Can raise Magnitudes to rational powers")
   }
 }
 
+TEST_CASE("can distinguish integral, rational, and irrational magnitudes")
+{
+  SECTION("Integer magnitudes are integral and rational")
+  {
+    auto check_rational_and_integral = [](Magnitude auto m) {
+      CHECK(is_integral(m));
+      CHECK(is_rational(m));
+    };
+    check_rational_and_integral(as_magnitude<1>());
+    check_rational_and_integral(as_magnitude<3>());
+    check_rational_and_integral(as_magnitude<8>());
+    check_rational_and_integral(as_magnitude<412>());
+  }
+
+  SECTION("Fractional magnitudes are rational, but not integral")
+  {
+    auto check_rational_but_not_integral = [](Magnitude auto m) {
+      CHECK(!is_integral(m));
+      CHECK(is_rational(m));
+    };
+    check_rational_but_not_integral(as_magnitude<ratio{1, 2}>());
+    check_rational_but_not_integral(as_magnitude<ratio{5, 8}>());
+  }
+}
+
 namespace detail {
 
+TEST_CASE("int_power computes integer powers") {
+  SECTION("handles floating point") {
+    check_same_type_and_value(int_power(0.123L, 0), 1.0L);
+    check_same_type_and_value(int_power(0.246f, 1), 0.246f);
+    check_same_type_and_value(int_power(0.5f, 3), 0.125f);
+    check_same_type_and_value(int_power(2.5, 4), 39.0625);
+
+    CHECK(std::is_same_v<long double, decltype(compute_base_power<double>(base_power{10, 20}))>);
+  }
+
+  SECTION("handles integral") {
+    check_same_type_and_value(int_power(8, 0), 1);
+    check_same_type_and_value(int_power(9L, 1), 9L);
+    check_same_type_and_value(int_power(2, 10), 1024);
+  }
+}
+
 TEST_CASE("Prime helper functions")
 {
   SECTION("find_first_factor()") {

From ed351a4ba3cb6a9f830d8f7691ba1ef7c037ed10 Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Fri, 28 Jan 2022 19:34:50 +0000
Subject: [PATCH 02/16] Avoid needing class to be completed

---
 src/core/include/units/magnitude.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index c33a61d3..7d4169d9 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -328,14 +328,16 @@ template<BasePower auto... BPs>
   requires (detail::is_base_power_pack_valid<BPs...>)
 struct magnitude {
   // Whether this magnitude represents an integer.
-  friend constexpr bool is_integral(magnitude) { return (detail::is_integral(BPs) && ...); }
+  static constexpr bool is_magnitude_integral = (detail::is_integral(BPs) && ...);
+  friend constexpr bool is_integral(magnitude) { return is_magnitude_integral; }
 
   // Whether this magnitude represents a rational number.
-  friend constexpr bool is_rational(magnitude) { return (detail::is_rational(BPs) && ...); }
+  static constexpr bool is_magnitude_rational = (detail::is_rational(BPs) && ...);
+  friend constexpr bool is_rational(magnitude) { return is_magnitude_rational; }
 
   // The value of this magnitude, expressed in a given type.
   template<typename T>
-    requires (is_integral(magnitude{}) || std::is_floating_point_v<T>)
+    requires is_magnitude_integral || std::is_floating_point_v<T>
   static constexpr T value = detail::checked_static_cast<T>(
       (detail::compute_base_power<T>(BPs) * ...));
 };

From 4c9a2c5c5ca61f8ac9e6e9196807cc284f1b4d2c Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Fri, 28 Jan 2022 19:52:35 +0000
Subject: [PATCH 03/16] Remove unsupported consteval

I had been using "consteval plus exception" as my way to get
"static_assert, but for parameters".  Since consteval doesn't work, then
I can't _guarantee_ that the functions won't be called at runtime.
However, I still think throwing exceptions is better, because it will
cause the desired compiler errors on every configuration.  (`assert`
often gets compiled out.)

Very much open to suggestion here.
---
 src/core/include/units/magnitude.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index 7d4169d9..902ff645 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -134,7 +134,9 @@ using widen_t = std::conditional_t<
 
 // Raise an arbitrary arithmetic type to a positive integer power at compile time.
 template<typename T> requires std::is_arithmetic_v<T>
-consteval T int_power(T base, std::integral auto exp){
+constexpr T int_power(T base, std::integral auto exp){
+  // As this function should only be called at compile time, the exceptions herein function as
+  // "parameter-compatible static_asserts", and should not result in exceptions at runtime.
   if (exp < 0) { throw std::invalid_argument{"int_power only supports positive integer powers"}; }
 
   // TODO(chogg): Unify this implementation with the one in pow.h.  That one takes its exponent as a
@@ -152,8 +154,6 @@ consteval T int_power(T base, std::integral auto exp){
   const auto result = square_root * square_root;
 
   if constexpr(std::is_unsigned_v<T>) {
-    // As this function can only be called at compile time, the exception functions as a
-    // "parameter-compatible static_assert", and does not result in exceptions at runtime.
     if (result / square_root != square_root) { throw std::overflow_error{"Unsigned wraparound"}; }
   }
 
@@ -162,13 +162,13 @@ consteval T int_power(T base, std::integral auto exp){
 
 
 template<typename T> requires std::is_arithmetic_v<T>
-consteval widen_t<T> compute_base_power(BasePower auto bp)
+constexpr widen_t<T> compute_base_power(BasePower auto bp)
 {
   // This utility can only handle integer powers.  To compute rational powers at compile time, we'll
   // need to write a custom function.
   //
-  // Note that since this function can only be called at compile time, the point of these exceptions
-  // is to act as "static_assert substitutes", not to throw actual exceptions at runtime.
+  // Note that since this function should only be called at compile time, the point of these
+  // exceptions is to act as "static_assert substitutes", not to throw actual exceptions at runtime.
   if (bp.power.den != 1) { throw std::invalid_argument{"Rational powers not yet supported"}; }
   if (bp.power.exp < 0) { throw std::invalid_argument{"Unsupported exp value"}; }
 
@@ -192,9 +192,9 @@ template<typename To, typename From>
   requires std::is_arithmetic_v<To>
     && std::is_arithmetic_v<From>
     && (std::is_integral_v<To> == std::is_integral_v<From>)
-consteval To checked_static_cast(From x) {
-  // This function can only ever be called at compile time.  The purpose of these exceptions is to
-  // produce compiler errors, because we cannot `static_assert` on function arguments.
+constexpr To checked_static_cast(From x) {
+  // This function should only ever be called at compile time.  The purpose of these exceptions is
+  // to produce compiler errors, because we cannot `static_assert` on function arguments.
   if constexpr (std::is_integral_v<To>) {
     if (std::cmp_less(x, std::numeric_limits<To>::min()) ||
         std::cmp_greater(x, std::numeric_limits<To>::max())) {

From e3a790667dfbeedac9afffc9eb089f689fc767c4 Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Fri, 28 Jan 2022 20:13:35 +0000
Subject: [PATCH 04/16] Guessing at what MSVC wants

---
 src/core/include/units/magnitude.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index 902ff645..2fa89e3f 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -337,7 +337,7 @@ struct magnitude {
 
   // The value of this magnitude, expressed in a given type.
   template<typename T>
-    requires is_magnitude_integral || std::is_floating_point_v<T>
+    requires (magnitude::is_magnitude_integral || std::is_floating_point_v<T>)
   static constexpr T value = detail::checked_static_cast<T>(
       (detail::compute_base_power<T>(BPs) * ...));
 };

From f2ba3cc18a4e36ff3e50300aa8e8d602f749a612 Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Fri, 28 Jan 2022 20:32:47 +0000
Subject: [PATCH 05/16] Use `in_range` for integer case

---
 src/core/include/units/magnitude.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index 2fa89e3f..14682235 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -196,8 +196,7 @@ constexpr To checked_static_cast(From x) {
   // This function should only ever be called at compile time.  The purpose of these exceptions is
   // to produce compiler errors, because we cannot `static_assert` on function arguments.
   if constexpr (std::is_integral_v<To>) {
-    if (std::cmp_less(x, std::numeric_limits<To>::min()) ||
-        std::cmp_greater(x, std::numeric_limits<To>::max())) {
+    if (!std::in_range<To>(x)) {
       throw std::invalid_argument{"Cannot represent magnitude in this type"};
     }
   } else {

From 243238b3fad7d7b75ddf0b4f34914b93db27642e Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Fri, 28 Jan 2022 20:41:08 +0000
Subject: [PATCH 06/16] Add suggested tests

---
 test/unit_test/runtime/magnitude_test.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/unit_test/runtime/magnitude_test.cpp b/test/unit_test/runtime/magnitude_test.cpp
index f41e1955..f1e73789 100644
--- a/test/unit_test/runtime/magnitude_test.cpp
+++ b/test/unit_test/runtime/magnitude_test.cpp
@@ -295,10 +295,12 @@ TEST_CASE("can distinguish integral, rational, and irrational magnitudes")
       CHECK(is_integral(m));
       CHECK(is_rational(m));
     };
+    check_rational_and_integral(magnitude<>{});
     check_rational_and_integral(as_magnitude<1>());
     check_rational_and_integral(as_magnitude<3>());
     check_rational_and_integral(as_magnitude<8>());
     check_rational_and_integral(as_magnitude<412>());
+    check_rational_and_integral(as_magnitude<ratio{1, 1}>());
   }
 
   SECTION("Fractional magnitudes are rational, but not integral")

From 2384276ae0111aee252dbec5acc18d30607d88cc Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Fri, 28 Jan 2022 20:44:29 +0000
Subject: [PATCH 07/16] Only test pi if float is smaller than long double

---
 test/unit_test/runtime/magnitude_test.cpp | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/test/unit_test/runtime/magnitude_test.cpp b/test/unit_test/runtime/magnitude_test.cpp
index f1e73789..00ccd43e 100644
--- a/test/unit_test/runtime/magnitude_test.cpp
+++ b/test/unit_test/runtime/magnitude_test.cpp
@@ -163,15 +163,17 @@ TEST_CASE("magnitude converts to numerical value")
 
   SECTION("pi to arbitrary power performs computations in most accurate type at compile time")
   {
-    constexpr auto pi_cubed = pi_to_the<3>();
+    if constexpr (sizeof(float) < sizeof(long double)) {
+      constexpr auto pi_cubed = pi_to_the<3>();
 
-    auto cube = [](auto x) { return x * x * x; };
-    constexpr auto via_float = cube(std::numbers::pi_v<float>);
-    constexpr auto via_long_double = static_cast<float>(cube(std::numbers::pi_v<long double>));
+      auto cube = [](auto x) { return x * x * x; };
+      constexpr auto via_float = cube(std::numbers::pi_v<float>);
+      constexpr auto via_long_double = static_cast<float>(cube(std::numbers::pi_v<long double>));
 
-    constexpr auto pi_cubed_value = pi_cubed.value<float>;
-    REQUIRE(pi_cubed_value != via_float);
-    CHECK(pi_cubed_value == via_long_double);
+      constexpr auto pi_cubed_value = pi_cubed.value<float>;
+      REQUIRE(pi_cubed_value != via_float);
+      CHECK(pi_cubed_value == via_long_double);
+    }
   }
 
   SECTION("Impossible requests are prevented at compile time")

From 811b6ba53d8ff726fe40f08d01250fd6c6b553c4 Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Fri, 28 Jan 2022 22:24:30 +0000
Subject: [PATCH 08/16] Try inlining the fold expression to satisfy (?) MSVC

---
 src/core/include/units/magnitude.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index 14682235..f6f3731f 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -336,7 +336,7 @@ struct magnitude {
 
   // The value of this magnitude, expressed in a given type.
   template<typename T>
-    requires (magnitude::is_magnitude_integral || std::is_floating_point_v<T>)
+    requires (std::is_floating_point_v<T> || (detail::is_integral(BPs) && ...))
   static constexpr T value = detail::checked_static_cast<T>(
       (detail::compute_base_power<T>(BPs) * ...));
 };

From d6eb25f07afec4dacf0f7d16e07e6d3981ddd059 Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Fri, 28 Jan 2022 22:42:23 +0000
Subject: [PATCH 09/16] Attempt to satisfy MSVC 14

```
test\unit_test\runtime\magnitude_test.cpp(143): error C2672:
'units::check_same_type_and_value': no matching overloaded function
found
```

Maybe it's confused by accessing the static member variable template
using dot-notation on an _instance_?  If so, let's see how it likes the
member-function syntax.
---
 src/core/include/units/magnitude.h        |  5 ++++
 test/unit_test/runtime/magnitude_test.cpp | 36 +++++++++++------------
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index f6f3731f..3a836b1b 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -339,6 +339,11 @@ struct magnitude {
     requires (std::is_floating_point_v<T> || (detail::is_integral(BPs) && ...))
   static constexpr T value = detail::checked_static_cast<T>(
       (detail::compute_base_power<T>(BPs) * ...));
+
+  // Trying to satisfy MSVC 14...
+  template<typename T>
+    requires (std::is_floating_point_v<T> || (detail::is_integral(BPs) && ...))
+  constexpr T get_value() const { return magnitude::value<T>; }
 };
 
 // Implementation for Magnitude concept (below).
diff --git a/test/unit_test/runtime/magnitude_test.cpp b/test/unit_test/runtime/magnitude_test.cpp
index 00ccd43e..20a370dd 100644
--- a/test/unit_test/runtime/magnitude_test.cpp
+++ b/test/unit_test/runtime/magnitude_test.cpp
@@ -140,25 +140,25 @@ TEST_CASE("magnitude converts to numerical value")
   SECTION("Positive integer powers of integer bases give integer values")
   {
     constexpr auto mag_412 = as_magnitude<412>();
-    check_same_type_and_value(mag_412.value<int>, 412);
-    check_same_type_and_value(mag_412.value<std::size_t>, std::size_t{412});
-    check_same_type_and_value(mag_412.value<float>, 412.0f);
-    check_same_type_and_value(mag_412.value<double>, 412.0);
+    check_same_type_and_value(mag_412.get_value<int>(), 412);
+    check_same_type_and_value(mag_412.get_value<std::size_t>(), std::size_t{412});
+    check_same_type_and_value(mag_412.get_value<float>(), 412.0f);
+    check_same_type_and_value(mag_412.get_value<double>(), 412.0);
   }
 
   SECTION("Negative integer powers of integer bases compute correct values")
   {
     constexpr auto mag_0p125 = as_magnitude<ratio{1, 8}>();
-    check_same_type_and_value(mag_0p125.value<float>, 0.125f);
-    check_same_type_and_value(mag_0p125.value<double>, 0.125);
+    check_same_type_and_value(mag_0p125.get_value<float>(), 0.125f);
+    check_same_type_and_value(mag_0p125.get_value<double>(), 0.125);
   }
 
   SECTION("pi to the 1 supplies correct values")
   {
     constexpr auto pi = pi_to_the<1>();
-    check_same_type_and_value(pi.value<float>, std::numbers::pi_v<float>);
-    check_same_type_and_value(pi.value<double>, std::numbers::pi_v<double>);
-    check_same_type_and_value(pi.value<long double>, std::numbers::pi_v<long double>);
+    check_same_type_and_value(pi.get_value<float>(), std::numbers::pi_v<float>);
+    check_same_type_and_value(pi.get_value<double>(), std::numbers::pi_v<double>);
+    check_same_type_and_value(pi.get_value<long double>(), std::numbers::pi_v<long double>);
   }
 
   SECTION("pi to arbitrary power performs computations in most accurate type at compile time")
@@ -170,7 +170,7 @@ TEST_CASE("magnitude converts to numerical value")
       constexpr auto via_float = cube(std::numbers::pi_v<float>);
       constexpr auto via_long_double = static_cast<float>(cube(std::numbers::pi_v<long double>));
 
-      constexpr auto pi_cubed_value = pi_cubed.value<float>;
+      constexpr auto pi_cubed_value = pi_cubed.get_value<float>();
       REQUIRE(pi_cubed_value != via_float);
       CHECK(pi_cubed_value == via_long_double);
     }
@@ -181,22 +181,22 @@ TEST_CASE("magnitude converts to numerical value")
     // Naturally, we cannot actually write a test to verify a compiler error.  But any of these can
     // be uncommented if desired to verify that it breaks the build.
 
-    // (void)as_magnitude<412>().value<int8_t>;
+    // (void)as_magnitude<412>().get_value<int8_t>();
 
     // Would work for pow<62>:
-    // (void)pow<63>(as_magnitude<2>()).value<int64_t>;
+    // (void)pow<63>(as_magnitude<2>()).get_value<int64_t>();
 
     // Would work for pow<63>:
-    // (void)pow<64>(as_magnitude<2>()).value<uint64_t>;
+    // (void)pow<64>(as_magnitude<2>()).get_value<uint64_t>();
 
-    (void)pow<308>(as_magnitude<10>()).value<double>; // Compiles, correctly.
-    // (void)pow<309>(as_magnitude<10>()).value<double>;
-    // (void)pow<3099>(as_magnitude<10>()).value<double>;
-    // (void)pow<3099999>(as_magnitude<10>()).value<double>;
+    (void)pow<308>(as_magnitude<10>()).get_value<double>(); // Compiles, correctly.
+    // (void)pow<309>(as_magnitude<10>()).get_value<double>();
+    // (void)pow<3099>(as_magnitude<10>()).get_value<double>();
+    // (void)pow<3099999>(as_magnitude<10>()).get_value<double>();
 
     auto sqrt_2 = pow<ratio{1, 2}>(as_magnitude<2>());
     CHECK(!is_integral(sqrt_2));
-    // (void)sqrt_2.value<int>;
+    // (void)sqrt_2.get_value<int>();
   }
 }
 

From b36bc2b58296ee87aa5f468df82304a4b0be1eff Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Sat, 29 Jan 2022 02:40:51 +0000
Subject: [PATCH 10/16] Turn get_value into free function

---
 src/core/include/units/magnitude.h        | 15 ++++++----
 test/unit_test/runtime/magnitude_test.cpp | 36 +++++++++++------------
 2 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index 3a836b1b..eed32e16 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -336,14 +336,9 @@ struct magnitude {
 
   // The value of this magnitude, expressed in a given type.
   template<typename T>
-    requires (std::is_floating_point_v<T> || (detail::is_integral(BPs) && ...))
+    requires (std::is_floating_point_v<T> || (std::is_integral_v<T> && is_magnitude_integral))
   static constexpr T value = detail::checked_static_cast<T>(
       (detail::compute_base_power<T>(BPs) * ...));
-
-  // Trying to satisfy MSVC 14...
-  template<typename T>
-    requires (std::is_floating_point_v<T> || (detail::is_integral(BPs) && ...))
-  constexpr T get_value() const { return magnitude::value<T>; }
 };
 
 // Implementation for Magnitude concept (below).
@@ -360,6 +355,14 @@ static constexpr bool is_magnitude<magnitude<BPs...>> = true;
 template<typename T>
 concept Magnitude = detail::is_magnitude<T>;
 
+/**
+ * @brief  Free-function access to the value of a Magnitude in a desired type.
+ *
+ * Can avoid the need for an unsightly `.template` keyword.
+ */
+template<typename T>
+T get_value(Magnitude auto m) { return m.template value<T>; }
+
 /**
  * @brief  Convert any positive integer to a Magnitude.
  *
diff --git a/test/unit_test/runtime/magnitude_test.cpp b/test/unit_test/runtime/magnitude_test.cpp
index 20a370dd..42449f95 100644
--- a/test/unit_test/runtime/magnitude_test.cpp
+++ b/test/unit_test/runtime/magnitude_test.cpp
@@ -140,25 +140,25 @@ TEST_CASE("magnitude converts to numerical value")
   SECTION("Positive integer powers of integer bases give integer values")
   {
     constexpr auto mag_412 = as_magnitude<412>();
-    check_same_type_and_value(mag_412.get_value<int>(), 412);
-    check_same_type_and_value(mag_412.get_value<std::size_t>(), std::size_t{412});
-    check_same_type_and_value(mag_412.get_value<float>(), 412.0f);
-    check_same_type_and_value(mag_412.get_value<double>(), 412.0);
+    check_same_type_and_value(get_value<int>(mag_412), 412);
+    check_same_type_and_value(get_value<std::size_t>(mag_412), std::size_t{412});
+    check_same_type_and_value(get_value<float>(mag_412), 412.0f);
+    check_same_type_and_value(get_value<double>(mag_412), 412.0);
   }
 
   SECTION("Negative integer powers of integer bases compute correct values")
   {
     constexpr auto mag_0p125 = as_magnitude<ratio{1, 8}>();
-    check_same_type_and_value(mag_0p125.get_value<float>(), 0.125f);
-    check_same_type_and_value(mag_0p125.get_value<double>(), 0.125);
+    check_same_type_and_value(get_value<float>(mag_0p125), 0.125f);
+    check_same_type_and_value(get_value<double>(mag_0p125), 0.125);
   }
 
   SECTION("pi to the 1 supplies correct values")
   {
     constexpr auto pi = pi_to_the<1>();
-    check_same_type_and_value(pi.get_value<float>(), std::numbers::pi_v<float>);
-    check_same_type_and_value(pi.get_value<double>(), std::numbers::pi_v<double>);
-    check_same_type_and_value(pi.get_value<long double>(), std::numbers::pi_v<long double>);
+    check_same_type_and_value(get_value<float>(pi), std::numbers::pi_v<float>);
+    check_same_type_and_value(get_value<double>(pi), std::numbers::pi_v<double>);
+    check_same_type_and_value(get_value<long double>(pi), std::numbers::pi_v<long double>);
   }
 
   SECTION("pi to arbitrary power performs computations in most accurate type at compile time")
@@ -170,7 +170,7 @@ TEST_CASE("magnitude converts to numerical value")
       constexpr auto via_float = cube(std::numbers::pi_v<float>);
       constexpr auto via_long_double = static_cast<float>(cube(std::numbers::pi_v<long double>));
 
-      constexpr auto pi_cubed_value = pi_cubed.get_value<float>();
+      constexpr auto pi_cubed_value = pi_cubed.value<float>;
       REQUIRE(pi_cubed_value != via_float);
       CHECK(pi_cubed_value == via_long_double);
     }
@@ -181,22 +181,22 @@ TEST_CASE("magnitude converts to numerical value")
     // Naturally, we cannot actually write a test to verify a compiler error.  But any of these can
     // be uncommented if desired to verify that it breaks the build.
 
-    // (void)as_magnitude<412>().get_value<int8_t>();
+    // (void)as_magnitude<412>().value<int8_t>;
 
     // Would work for pow<62>:
-    // (void)pow<63>(as_magnitude<2>()).get_value<int64_t>();
+    // (void)pow<63>(as_magnitude<2>()).value<int64_t>;
 
     // Would work for pow<63>:
-    // (void)pow<64>(as_magnitude<2>()).get_value<uint64_t>();
+    // (void)pow<64>(as_magnitude<2>()).value<uint64_t>;
 
-    (void)pow<308>(as_magnitude<10>()).get_value<double>(); // Compiles, correctly.
-    // (void)pow<309>(as_magnitude<10>()).get_value<double>();
-    // (void)pow<3099>(as_magnitude<10>()).get_value<double>();
-    // (void)pow<3099999>(as_magnitude<10>()).get_value<double>();
+    (void)pow<308>(as_magnitude<10>()).value<double>; // Compiles, correctly.
+    // (void)pow<309>(as_magnitude<10>()).value<double>;
+    // (void)pow<3099>(as_magnitude<10>()).value<double>;
+    // (void)pow<3099999>(as_magnitude<10>()).value<double>;
 
     auto sqrt_2 = pow<ratio{1, 2}>(as_magnitude<2>());
     CHECK(!is_integral(sqrt_2));
-    // (void)sqrt_2.get_value<int>();
+    // (void)sqrt_2.value<int>;
   }
 }
 

From 408ad16528ba87e9b1bfc91b146a4502297409e5 Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Sat, 29 Jan 2022 03:19:30 +0000
Subject: [PATCH 11/16] Spell out fold expression, again

---
 src/core/include/units/magnitude.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index eb6278e4..9e203848 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -336,7 +336,9 @@ struct magnitude {
 
   // The value of this magnitude, expressed in a given type.
   template<typename T>
-    requires (std::is_floating_point_v<T> || (std::is_integral_v<T> && is_magnitude_integral))
+    requires (
+        std::is_floating_point_v<T>
+        || (std::is_integral_v<T> && (detail::is_integral(BPs) && ...)))
   static constexpr T value = detail::checked_static_cast<T>(
       (detail::compute_base_power<T>(BPs) * ...));
 };

From 9593ca04d7e789917fa95fa31a90e33046ea279f Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Sat, 29 Jan 2022 03:47:16 +0000
Subject: [PATCH 12/16] Refer to class instead

---
 src/core/include/units/magnitude.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index 9e203848..9c57baa8 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -363,7 +363,7 @@ concept Magnitude = detail::is_magnitude<T>;
  * Can avoid the need for an unsightly `.template` keyword.
  */
 template<typename T>
-T get_value(Magnitude auto m) { return m.template value<T>; }
+T get_value(Magnitude auto m) { return decltype(m)::template value<T>; }
 
 /**
  * @brief  A base to represent pi.

From 7615de4720352bd89da164bb5b04d7c0d437e2be Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Sun, 6 Feb 2022 21:06:53 +0000
Subject: [PATCH 13/16] See if passing by const& will satisfy all compilers

The only reason we had the `is_magnitude_integral` type members before
is because the compiler complained about incomplete types.  Passing by
`const&` might eliminate this need.
---
 src/core/include/units/magnitude.h | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index 9c57baa8..dfd9632a 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -327,12 +327,10 @@ template<BasePower auto... BPs>
   requires (detail::is_base_power_pack_valid<BPs...>)
 struct magnitude {
   // Whether this magnitude represents an integer.
-  static constexpr bool is_magnitude_integral = (detail::is_integral(BPs) && ...);
-  friend constexpr bool is_integral(magnitude) { return is_magnitude_integral; }
+  friend constexpr bool is_integral(const magnitude&) { return (detail::is_integral(BPs) && ...); }
 
   // Whether this magnitude represents a rational number.
-  static constexpr bool is_magnitude_rational = (detail::is_rational(BPs) && ...);
-  friend constexpr bool is_rational(magnitude) { return is_magnitude_rational; }
+  friend constexpr bool is_rational(const magnitude&) { return (detail::is_rational(BPs) && ...); }
 
   // The value of this magnitude, expressed in a given type.
   template<typename T>
@@ -363,7 +361,7 @@ concept Magnitude = detail::is_magnitude<T>;
  * Can avoid the need for an unsightly `.template` keyword.
  */
 template<typename T>
-T get_value(Magnitude auto m) { return decltype(m)::template value<T>; }
+constexpr T get_value(Magnitude auto m) { return decltype(m)::template value<T>; }
 
 /**
  * @brief  A base to represent pi.

From 7e4ab4206f7c693c40cdcc90ecae1e970621fd34 Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Sun, 6 Feb 2022 21:53:05 +0000
Subject: [PATCH 14/16] Remove `.value`; provide free function only

This is a cleaner interface.

I also checked all of the commented-out test cases.
---
 src/core/include/units/magnitude.h        | 41 +++++++++++------------
 test/unit_test/runtime/magnitude_test.cpp | 18 +++++-----
 2 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index dfd9632a..55b295cf 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -139,6 +139,14 @@ constexpr T int_power(T base, std::integral auto exp){
   // "parameter-compatible static_asserts", and should not result in exceptions at runtime.
   if (exp < 0) { throw std::invalid_argument{"int_power only supports positive integer powers"}; }
 
+  constexpr auto checked_multiply = [] (auto a, auto b) {
+    const auto result = a * b;
+    if (result / a != b) { throw std::overflow_error{"Wraparound detected"}; }
+    return result;
+  };
+
+  constexpr auto checked_square = [checked_multiply] (auto a) { return checked_multiply(a, a); };
+
   // TODO(chogg): Unify this implementation with the one in pow.h.  That one takes its exponent as a
   // template parameter, rather than a function parameter.
 
@@ -147,17 +155,10 @@ constexpr T int_power(T base, std::integral auto exp){
   }
 
   if (exp % 2 == 1) {
-    return base * int_power(base, exp - 1);
+    return checked_multiply(base, int_power(base, exp - 1));
   }
 
-  const auto square_root = int_power(base, exp / 2);
-  const auto result = square_root * square_root;
-
-  if constexpr(std::is_unsigned_v<T>) {
-    if (result / square_root != square_root) { throw std::overflow_error{"Unsigned wraparound"}; }
-  }
-
-  return result;
+  return checked_square(int_power(base, exp / 2));
 }
 
 
@@ -331,14 +332,6 @@ struct magnitude {
 
   // Whether this magnitude represents a rational number.
   friend constexpr bool is_rational(const magnitude&) { return (detail::is_rational(BPs) && ...); }
-
-  // The value of this magnitude, expressed in a given type.
-  template<typename T>
-    requires (
-        std::is_floating_point_v<T>
-        || (std::is_integral_v<T> && (detail::is_integral(BPs) && ...)))
-  static constexpr T value = detail::checked_static_cast<T>(
-      (detail::compute_base_power<T>(BPs) * ...));
 };
 
 // Implementation for Magnitude concept (below).
@@ -356,12 +349,16 @@ template<typename T>
 concept Magnitude = detail::is_magnitude<T>;
 
 /**
- * @brief  Free-function access to the value of a Magnitude in a desired type.
- *
- * Can avoid the need for an unsightly `.template` keyword.
+ * @brief  The value of a Magnitude in a desired type T.
  */
-template<typename T>
-constexpr T get_value(Magnitude auto m) { return decltype(m)::template value<T>; }
+template<typename T, BasePower auto... BPs>
+  requires (std::is_floating_point_v<T> || (std::is_integral_v<T> && is_integral(magnitude<BPs...>{})))
+constexpr T get_value(const magnitude<BPs...> &) {
+  // Force the expression to be evaluated in a constexpr context, to catch, e.g., overflow.
+  constexpr auto result = detail::checked_static_cast<T>((detail::compute_base_power<T>(BPs) * ...));
+
+  return result;
+}
 
 /**
  * @brief  A base to represent pi.
diff --git a/test/unit_test/runtime/magnitude_test.cpp b/test/unit_test/runtime/magnitude_test.cpp
index 42449f95..76dcb16c 100644
--- a/test/unit_test/runtime/magnitude_test.cpp
+++ b/test/unit_test/runtime/magnitude_test.cpp
@@ -170,7 +170,7 @@ TEST_CASE("magnitude converts to numerical value")
       constexpr auto via_float = cube(std::numbers::pi_v<float>);
       constexpr auto via_long_double = static_cast<float>(cube(std::numbers::pi_v<long double>));
 
-      constexpr auto pi_cubed_value = pi_cubed.value<float>;
+      constexpr auto pi_cubed_value = get_value<float>(pi_cubed);
       REQUIRE(pi_cubed_value != via_float);
       CHECK(pi_cubed_value == via_long_double);
     }
@@ -181,22 +181,22 @@ TEST_CASE("magnitude converts to numerical value")
     // Naturally, we cannot actually write a test to verify a compiler error.  But any of these can
     // be uncommented if desired to verify that it breaks the build.
 
-    // (void)as_magnitude<412>().value<int8_t>;
+    // get_value<int8_t>(as_magnitude<412>());
 
     // Would work for pow<62>:
-    // (void)pow<63>(as_magnitude<2>()).value<int64_t>;
+    // get_value<int64_t>(pow<63>(as_magnitude<2>()));
 
     // Would work for pow<63>:
-    // (void)pow<64>(as_magnitude<2>()).value<uint64_t>;
+    // get_value<uint64_t>(pow<64>(as_magnitude<2>()));
 
-    (void)pow<308>(as_magnitude<10>()).value<double>; // Compiles, correctly.
-    // (void)pow<309>(as_magnitude<10>()).value<double>;
-    // (void)pow<3099>(as_magnitude<10>()).value<double>;
-    // (void)pow<3099999>(as_magnitude<10>()).value<double>;
+    get_value<double>(pow<308>(as_magnitude<10>())); // Compiles, correctly.
+    // get_value<double>(pow<309>(as_magnitude<10>()));
+    // get_value<double>(pow<3099>(as_magnitude<10>()));
+    // get_value<double>(pow<3099999>(as_magnitude<10>()));
 
     auto sqrt_2 = pow<ratio{1, 2}>(as_magnitude<2>());
     CHECK(!is_integral(sqrt_2));
-    // (void)sqrt_2.value<int>;
+    // get_value<int>(sqrt_2);
   }
 }
 

From a0ae08746f2e21a82415b70e7248605df5707ef2 Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Sun, 6 Feb 2022 21:56:00 +0000
Subject: [PATCH 15/16] Restore mistakenly removed comment

---
 src/core/include/units/magnitude.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index 55b295cf..b95e5558 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -458,6 +458,12 @@ template<std::intmax_t N>
 static constexpr auto prime_factorization_v = prime_factorization<N>::value;
 } // namespace detail
 
+/**
+ * @brief  Convert any positive integer to a Magnitude.
+ *
+ * This will be the main way end users create Magnitudes.  They should rarely (if ever) create a magnitude<...> by
+ * manually adding base powers.
+ */
 template<ratio R>
   requires (R.num > 0)
 constexpr Magnitude auto as_magnitude() {

From 7820b3ef92d46abfd7f9ba2fc1e4e9cdda95eaa7 Mon Sep 17 00:00:00 2001
From: Chip Hogg <chogg@aurora.tech>
Date: Sun, 6 Feb 2022 22:11:42 +0000
Subject: [PATCH 16/16] Clean up a few requires clauses

---
 src/core/include/units/magnitude.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/core/include/units/magnitude.h b/src/core/include/units/magnitude.h
index b95e5558..77de58b2 100644
--- a/src/core/include/units/magnitude.h
+++ b/src/core/include/units/magnitude.h
@@ -295,7 +295,7 @@ pairwise_all(T) -> pairwise_all<T>;
 
 // Check whether a sequence of (possibly heterogeneously typed) values are strictly increasing.
 template<typename... Ts>
-  requires ((std::is_signed_v<Ts> && ...))
+  requires (std::is_signed_v<Ts> && ...)
 constexpr bool strictly_increasing(Ts&&... ts) {
   return pairwise_all{std::less{}}(std::forward<Ts>(ts)...);
 }
@@ -325,7 +325,7 @@ constexpr bool is_integral(BasePower auto bp) {
  * rational powers, and compare for equality.
  */
 template<BasePower auto... BPs>
-  requires (detail::is_base_power_pack_valid<BPs...>)
+  requires detail::is_base_power_pack_valid<BPs...>
 struct magnitude {
   // Whether this magnitude represents an integer.
   friend constexpr bool is_integral(const magnitude&) { return (detail::is_integral(BPs) && ...); }
@@ -352,7 +352,7 @@ concept Magnitude = detail::is_magnitude<T>;
  * @brief  The value of a Magnitude in a desired type T.
  */
 template<typename T, BasePower auto... BPs>
-  requires (std::is_floating_point_v<T> || (std::is_integral_v<T> && is_integral(magnitude<BPs...>{})))
+  requires std::is_floating_point_v<T> || (std::is_integral_v<T> && is_integral(magnitude<BPs...>{}))
 constexpr T get_value(const magnitude<BPs...> &) {
   // Force the expression to be evaluated in a constexpr context, to catch, e.g., overflow.
   constexpr auto result = detail::checked_static_cast<T>((detail::compute_base_power<T>(BPs) * ...));