Add XKCD example

examples/xkcd2585.ins

@@ -0,0 +1,88 @@
+# https://xkcd.com/2585/
+#
+# I can ride my bike at 45 mph.
+# If you round.
+
+17 mph
+
+ans -> meters/sec    // round
+ans -> knots         // round
+ans -> fathoms/sec   // round
+ans -> furlongs/min  // round
+ans -> fathoms/sec   // round
+ans -> kph           // round
+ans -> knots         // round
+ans -> kph           // round
+ans -> furlongs/hour // round
+ans -> mi/h          // round       # TODO: replace mi/h with mph. see below for details
+ans -> m/s           // round
+ans -> furlongs/min  // round
+ans -> yards/sec     // round
+ans -> fathoms/sec   // round
+ans -> m/s           // round
+ans -> mph           // round
+ans -> furlongs/min  // round
+ans -> knots         // round
+ans -> yards/sec     // round
+ans -> fathoms/sec   // round
+ans -> knots         // round
+ans -> furlongs/min  // round
+ans -> mph           // round
+
+assert_eq(ans, 45 mph)
+
+
+# Unfortunately, if we replace mi/h above with mph, the test fails.
+# The reason lies in the '204 furlongs/hour -> mph' conversion.
+# 204 furlongs are 25.5 miles (exact). 204 furlongs/hour -> mph is
+# therefore supposed to yield 26 mph after rounding. However, in our
+# primitive `Quantity::convert_to` implementation, we first convert
+# to the corresponding base unit (m/s) and then back:
+#
+#    furlongs/hour -> m/s -> mph
+#
+# Minor floating point inaccuracies then lead to a result which is
+# slightly less than 25.5 mph. After rounding, this becomes 25 mph.
+#
+# Improving the conversion algorithm slightly by removing common
+# unit factors (here: 1/hour), improved the situation and now leads
+# to the correct result when using mi/h (mile/hour). However, if we
+# use the `mph` shorthand, our common-unit-factor computation fails
+# and still leads to the wrong 25 mph result.
+#
+# A proper solution to this would probably build a full DAG for all
+# unit definitions of a single physical dimension. The conversion
+# algorithm could then be changed to use shortest paths in those
+# graphs. For example: If both miles and furlongs were defined via
+# the unit foot, we could use foot/hour as the 'base' unit for the
+# conversion instead of meter/second:
+#
+#
+#                meter       second
+#
+#                  ▲            ▲
+#                  │            │
+#                  │            │
+#                  │            │
+#
+#                inch        minute
+#
+#                  ▲            ▲
+#                  │            │
+#                  │            │
+#                  │            │
+#
+#         ┌────► foot         hour
+#         │
+#         │        ▲
+#         │        │
+#         │        │
+#         │        │
+#
+#       mile    furlong
+#
+#
+# Another, potentially simpler solution to this problem could be to
+# introduce a proper type for rationals. This way, floating point
+# inaccuracies would only appear for units with irrational defining
+# conversion factors, like `unit degree = pi / 180 × radian`.

insect/src/quantity.rs

@@ -4,7 +4,7 @@ use crate::unit::{Unit, UnitFactor};
 
 use itertools::Itertools;
 use num_rational::Ratio;
-use num_traits::FromPrimitive;
+use num_traits::{FromPrimitive, Zero};
 use thiserror::Error;
 
 #[derive(Clone, Debug, Error, PartialEq, Eq)]
@@ -51,13 +51,52 @@ impl Quantity {
         if &self.unit == target_unit || self.is_zero() {
             Ok(Quantity::new(self.value, target_unit.clone()))
         } else {
+            // Remove common unit factors to reduce unnecessary conversion procedures
+            // For example: when converting from km/hour to mile/hour, there is no need
+            // to also perform the hour->second conversion, which would be needed, as
+            // we go back to base units for now. Removing common factors is just one
+            // heuristic, but it would be better to solve this in a more general way.
+            // For more details on this problem, see `examples/xkcd2585.ins`.
+            let mut common_unit_factors = Unit::scalar();
+            let target_unit_canonicalized = target_unit.canonicalized();
+            for factor in self.unit.canonicalized().iter() {
+                if let Some(other_factor) = target_unit_canonicalized
+                    .iter()
+                    .find(|&f| factor.prefix == f.prefix && factor.unit_id == f.unit_id)
+                {
+                    if factor.exponent > Ratio::zero() && other_factor.exponent > Ratio::zero() {
+                        common_unit_factors = common_unit_factors
+                            * Unit::from_factor(UnitFactor {
+                                exponent: std::cmp::min(factor.exponent, other_factor.exponent),
+                                ..factor.clone()
+                            });
+                    } else if factor.exponent < Ratio::zero()
+                        && other_factor.exponent < Ratio::zero()
+                    {
+                        common_unit_factors = common_unit_factors
+                            * Unit::from_factor(UnitFactor {
+                                exponent: std::cmp::max(factor.exponent, other_factor.exponent),
+                                ..factor.clone()
+                            });
+                    }
+                }
+            }
+
+            let target_unit_reduced =
+                (target_unit.clone() / common_unit_factors.clone()).canonicalized();
+            let own_unit_reduced =
+                (self.unit.clone() / common_unit_factors.clone()).canonicalized();
+
             let (target_base_unit_representation, factor) =
-                target_unit.to_base_unit_representation();
+                target_unit_reduced.to_base_unit_representation();
 
-            let quantity_base_unit_representation = self.to_base_unit_representation();
-            let self_base_unit_representation = quantity_base_unit_representation.unit();
+            let quantity_base_unit_representation = (self.clone()
+                / Quantity::from_unit(common_unit_factors))
+            .unwrap()
+            .to_base_unit_representation();
+            let own_base_unit_representation = own_unit_reduced.to_base_unit_representation().0;
 
-            if self_base_unit_representation == &target_base_unit_representation {
+            if own_base_unit_representation == target_base_unit_representation {
                 Ok(Quantity::new(
                     *quantity_base_unit_representation.unsafe_value() / factor,
                     target_unit.clone(),