Fix derivatives giving bad results

src/derivatives.rs

@@ -0,0 +1,58 @@
+use std::collections::{BTreeMap, BinaryHeap};
+use eval::Context;
+use std::rc::Rc;
+use number::{Number, Unit};
+use std::cmp;
+
+#[derive(PartialEq, Eq, Debug)]
+pub struct Deriv(pub usize, pub Vec<Rc<String>>);
+
+impl cmp::PartialOrd for Deriv {
+    fn partial_cmp(&self, other: &Deriv) -> Option<cmp::Ordering> {
+        Some(self.0.cmp(&other.0).reverse())
+    }
+}
+
+impl cmp::Ord for Deriv {
+    fn cmp(&self, other: &Deriv) -> cmp::Ordering {
+        self.partial_cmp(other).unwrap()
+    }
+}
+
+pub fn derivatives(ctx: &Context, value: &Number, aliases: &BTreeMap<Unit, Rc<String>>)
+                   -> BinaryHeap<Deriv> {
+    if value.1.len() == 0 {
+        let mut map = BinaryHeap::new();
+        map.push(Deriv(0, vec![]));
+        return map;
+    }
+    let mut candidates: BinaryHeap<Deriv> = BinaryHeap::new();
+    let value_score = value.complexity_score();
+    for (unit, name) in aliases.iter().rev() {
+        use gmp::mpq::Mpq;
+
+        let res = (value / &Number(Mpq::one(), unit.clone())).unwrap();
+        //if res.1.len() >= value.1.len() {
+        let score = res.complexity_score();
+        // we are not making the unit any simpler
+        if score >= value_score {
+            continue
+        }
+        let res = derivatives(ctx, &res, aliases);
+        for Deriv(score, mut vec) in res {
+            vec.push(name.clone());
+            candidates.push(Deriv(score + 1, vec));
+        }
+        let mut next = BinaryHeap::new();
+        for _ in 0..10 {
+            if let Some(v) = candidates.pop() {
+                next.push(v);
+            } else {
+                break
+            }
+        }
+        candidates = next;
+    }
+    assert!(candidates.len() <= 10);
+    candidates
+}

src/eval.rs

@@ -7,6 +7,7 @@ use date;
 use unit_defs::DatePattern;
 use std::ops::{Add, Div, Mul, Neg, Sub};
 use std::rc::Rc;
+use derivatives::{derivatives, Deriv};
 
 #[derive(Clone)]
 pub enum Value {
@@ -177,40 +178,6 @@ impl Context {
         })
     }
 
-    pub fn derivatives(&self, value: &Number, aliases: &BTreeMap<Unit, Rc<String>>)
-                       -> BTreeMap<usize, Vec<Rc<String>>> {
-        if value.1.len() == 0 {
-            let mut map = BTreeMap::new();
-            map.insert(0, vec![]);
-            return map;
-        }
-        let mut candidates: BTreeMap<usize, Vec<Rc<String>>> = BTreeMap::new();
-        let value_score = value.complexity_score();
-        for (unit, name) in aliases.iter().rev() {
-            use gmp::mpq::Mpq;
-
-            let res = (value / &Number(Mpq::one(), unit.clone())).unwrap();
-            //if res.1.len() >= value.1.len() {
-            let score = res.complexity_score();
-            // we are not making the unit any simpler
-            if score >= value_score {
-                continue
-            }
-            let res = self.derivatives(&res, aliases);
-            for (score, mut vec) in res {
-                vec.push(name.clone());
-                // more complicated than decomposition to base units
-                /*if score + 1 > value.1.len() {
-                    continue
-                }*/
-                candidates.insert(score + 1, vec);
-            }
-            candidates = candidates.into_iter().take(10).collect();
-        }
-        assert!(candidates.len() <= 10);
-        candidates
-    }
-
     /// Describes a value's unit, gives true if the unit is reciprocal
     /// (e.g. you should prefix "1.0 / " or replace "multiply" with
     /// "divide" when rendering it).
@@ -603,8 +570,8 @@ impl Context {
                 let aliases = self.aliases.iter()
                     .map(|(a, b)| (a.clone(), Rc::new(b.clone())))
                     .collect::<BTreeMap<_, _>>();
-                let derivs = self.derivatives(&val, &aliases);
-                let derivs = derivs.into_iter().map(|(_score, names)| {
+                let derivs = derivatives(self, &val, &aliases);
+                let derivs = derivs.into_iter().map(|Deriv(_score, names)| {
                     let first = names.first().cloned();
                     names.into_iter().skip(1).fold(
                         first.map(|x| (**x).to_owned()).unwrap_or(String::new()),

src/lib.rs

@@ -39,6 +39,7 @@ pub mod unit_defs;
 pub mod eval;
 pub mod number;
 pub mod date;
+pub mod derivatives;
 
 pub use number::Number;
 pub use eval::{Context, Value};