Merge pull request #187 from MarekSpodBiedry/issue-57

Compare numbers at 15 significant digits like Excel (#57)

crates/core/src/lib.rs

@@ -10,4 +10,4 @@ pub mod value;
 
 pub use addr::{CellRef, MAX_COLS, MAX_ROWS, RangeRef, col_to_letters, letters_to_col};
 pub use date::DateSystem;
-pub use value::{Array, CellError, Value, compare, compare_text, number_to_text, sort_compare};
+pub use value::{Array, CellError, Value, compare, compare_numbers, compare_text, number_to_text, sort_compare};

crates/core/src/value.rs

@@ -288,13 +288,13 @@ impl Value {
 pub fn compare(a: &Value, b: &Value) -> Ordering {
     match (a, b) {
         (Value::Empty, Value::Empty) => Ordering::Equal,
-        (Value::Empty, Value::Number(n)) => 0.0f64.partial_cmp(n).unwrap_or(Ordering::Equal),
-        (Value::Number(n), Value::Empty) => n.partial_cmp(&0.0).unwrap_or(Ordering::Equal),
+        (Value::Empty, Value::Number(n)) => compare_numbers(0.0, *n),
+        (Value::Number(n), Value::Empty) => compare_numbers(*n, 0.0),
         (Value::Empty, Value::Text(t)) => "".cmp(&t.to_lowercase() as &str),
         (Value::Text(t), Value::Empty) => (t.to_lowercase() as String).as_str().cmp(""),
         (Value::Empty, Value::Bool(b)) => false.cmp(b),
         (Value::Bool(b), Value::Empty) => b.cmp(&false),
-        (Value::Number(x), Value::Number(y)) => x.partial_cmp(y).unwrap_or(Ordering::Equal),
+        (Value::Number(x), Value::Number(y)) => compare_numbers(*x, *y),
         (Value::Text(x), Value::Text(y)) => compare_text(x, y),
         (Value::Bool(x), Value::Bool(y)) => x.cmp(y),
         (Value::Error(x), Value::Error(y)) => x.code().cmp(&y.code()),
@@ -302,6 +302,23 @@ pub fn compare(a: &Value, b: &Value) -> Ordering {
     }
 }
 
+/// Compares numbers the way Excel does: both are rounded to 15 significant digits first, so binary
+/// noise such as `0.1+0.2` versus `0.3` counts as equal.
+pub fn compare_numbers(a: f64, b: f64) -> Ordering {
+    fn round15(x: f64) -> f64 {
+        if !x.is_finite() || x == 0.0 { x } else { format!("{x:.14e}").parse().unwrap_or(x) }
+    }
+    if a == b {
+        return Ordering::Equal;
+    }
+    // Numbers further apart than the 15th digit can't round together, and rounding keeps their
+    // order: skip the text round trip (this is on the sort and lookup hot path).
+    if (a - b).abs() > a.abs().max(b.abs()) * 1e-13 {
+        return a.partial_cmp(&b).unwrap_or(Ordering::Equal);
+    }
+    round15(a).partial_cmp(&round15(b)).unwrap_or(Ordering::Equal)
+}
+
 /// Case-insensitive text order (Unicode lowercase).
 pub fn compare_text(a: &str, b: &str) -> Ordering {
     let mut ai = a.chars().flat_map(char::to_lowercase);
@@ -410,6 +427,17 @@ mod tests {
         assert_eq!(CellError::parse("#div/0!"), Some(CellError::Div0));
     }
 
+    #[test]
+    fn numbers_compare_at_15_significant_digits() {
+        let num = Value::Number;
+        assert_eq!(compare(&num(0.1 + 0.2), &num(0.3)), Ordering::Equal);
+        assert_eq!(compare(&num(1.1 * 3.0), &num(3.3)), Ordering::Equal);
+        assert_eq!(compare(&num(1.0), &num(1.0 + 1e-16)), Ordering::Equal);
+        assert_ne!(compare(&num(0.1 + 0.2), &num(0.3)), Ordering::Greater);
+        assert_eq!(compare(&num(1.0), &num(1.00000000000001)), Ordering::Less);
+        assert_eq!(compare(&num(1.0), &num(2.0)), Ordering::Less);
+    }
+
     #[test]
     fn arrays() {
         let a = Array::new(2, 2, vec![1.0.into(), 2.0.into(), 3.0.into(), 4.0.into()]).unwrap();

crates/functions/src/criteria.rs

@@ -3,7 +3,7 @@
 use std::cmp::Ordering;
 
 use gridcraft_core::parse::parse_number_text;
-use gridcraft_core::{CellError, Value, compare_text};
+use gridcraft_core::{CellError, Value, compare_numbers, compare_text};
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 enum Op {
@@ -113,7 +113,7 @@ impl Criterion {
                     _ => None,
                 };
                 match cell {
-                    Some(x) => cmp_op(self.op, x.partial_cmp(n).unwrap_or(Ordering::Equal)),
+                    Some(x) => cmp_op(self.op, compare_numbers(x, *n)),
                     None => self.op == Op::Ne,
                 }
             }
@@ -252,7 +252,7 @@ pub(crate) fn lookup_equal(needle: &Value, hay: &Value, wild: bool) -> bool {
                 compare_text(p, t) == Ordering::Equal
             }
         }
-        (Value::Number(a), Value::Number(b)) => a == b,
+        (Value::Number(a), Value::Number(b)) => compare_numbers(*a, *b) == Ordering::Equal,
         (Value::Bool(a), Value::Bool(b)) => a == b,
         (Value::Error(a), Value::Error(b)) => a == b,
         (Value::Empty, Value::Empty) => true,
@@ -294,6 +294,8 @@ mod tests {
         assert!(!m("<b", Value::Number(1.0)));
         assert!(m("#N/A", Value::Error(CellError::NA)));
         assert!(Criterion::parse(&Value::Number(3.0)).matches(&Value::Number(3.0)));
+        assert!(Criterion::parse(&Value::Number(0.3)).matches(&Value::Number(0.1 + 0.2)));
+        assert!(lookup_equal(&Value::Number(0.3), &Value::Number(0.1 + 0.2), false));
         assert!(m("<>a*", Value::from("bcd")));
         assert!(m(">1/1/2020", Value::Number(44000.0)));
     }