From a020ac669f91f13a3785e45c87d3fdf2602f5987 Mon Sep 17 00:00:00 2001 From: "Claude (dbsp-backend)" Date: Sat, 10 Oct 2026 15:14:20 -0700 Subject: [PATCH] feat(incremental): Sum and Avg through an exact sum in both backends (improvise-hrjz) Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- Cargo.lock | 2 + Cargo.nix | 8 + crates/improvise-core/Cargo.toml | 2 + crates/improvise-core/src/model/types.rs | 14 +- crates/improvise-incremental/Cargo.toml | 1 + crates/improvise-incremental/src/group.rs | 86 ++++-- .../improvise-incremental/src/incremental.rs | 13 +- crates/improvise-incremental/src/shapes.rs | 79 +++-- crates/improvise-incremental/tests/exact.rs | 139 +++++++++ crates/improvise-incremental/tests/measure.rs | 57 +++- crates/improvise-incremental/tests/random.rs | 1 + .../tests/support/mod.rs | 1 + crates/numtower/src/lib.rs | 2 +- crates/numtower/src/sum.rs | 39 +++ crates/numtower/tests/float_sum.rs | 78 +++++ harness/opfs-contract/Cargo.lock | 286 ++++++++++++++++++ 16 files changed, 732 insertions(+), 76 deletions(-) create mode 100644 crates/improvise-incremental/tests/exact.rs create mode 100644 crates/numtower/tests/float_sum.rs diff --git a/Cargo.lock b/Cargo.lock index 1df702f8..17a1b2ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -924,6 +924,7 @@ dependencies = [ "anyhow", "improvise-formula", "indexmap", + "numtower", "proptest", "serde", "serde_json", @@ -959,6 +960,7 @@ version = "0.1.0" dependencies = [ "improvise-core", "improvise-io", + "numtower", "proptest", "zset", ] diff --git a/Cargo.nix b/Cargo.nix index 514364fe..3dcaa531 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -3063,6 +3063,10 @@ rec { name = "indexmap"; packageId = "indexmap"; } + { + name = "numtower"; + packageId = "numtower"; + } { name = "serde"; packageId = "serde"; @@ -3168,6 +3172,10 @@ rec { name = "improvise-core"; packageId = "improvise-core"; } + { + name = "numtower"; + packageId = "numtower"; + } { name = "zset"; packageId = "zset"; diff --git a/crates/improvise-core/Cargo.toml b/crates/improvise-core/Cargo.toml index ee52b099..4910b9c9 100644 --- a/crates/improvise-core/Cargo.toml +++ b/crates/improvise-core/Cargo.toml @@ -8,6 +8,8 @@ repository = "https://github.com/fiddlerwoaroof/improvise" [dependencies] improvise-formula = { path = "../improvise-formula" } +# ADR 009 clause 8: every Sum and Avg is exact, in both backends. +numtower = { path = "../numtower" } anyhow = "1" indexmap = "2" serde = { version = "1", features = ["derive"] } diff --git a/crates/improvise-core/src/model/types.rs b/crates/improvise-core/src/model/types.rs index 77be6b50..51bfd148 100644 --- a/crates/improvise-core/src/model/types.rs +++ b/crates/improvise-core/src/model/types.rs @@ -15,12 +15,20 @@ pub const MAX_CATEGORIES: usize = 24; /// They must agree; why, and why 1e-10: context/code/core.md §4. const FLOAT_EQ_EPSILON: f64 = 1e-10; +/// The sum correctly rounded once, whatever the order (ADR 009 clause 8; `-0.0` only if every +/// addend is, as `Sum for f64` gives). +fn exact_sum(values: &[f64]) -> f64 { + let mut sum = numtower::FloatSum::new(); + values.iter().for_each(|v| sum.add(*v, 1)); + sum.value() +} + /// Reduce raw values. `None` on empty: an empty cell reads as **absent**, not /// as zero. Both aggregation paths share it (improvise-kdp, core.md §2). fn apply_agg(values: &[f64], func: &AggFunc) -> Option { (!values.is_empty()).then(|| match func { - AggFunc::Sum => values.iter().sum(), - AggFunc::Avg => values.iter().sum::() / values.len() as f64, + AggFunc::Sum => exact_sum(values), + AggFunc::Avg => exact_sum(values) / values.len() as f64, AggFunc::Min => values.iter().copied().fold(f64::INFINITY, f64::min), AggFunc::Max => values.iter().copied().fold(f64::NEG_INFINITY, f64::max), AggFunc::Count => values.len() as f64, @@ -78,7 +86,7 @@ fn eval_agg( .collect::, String>>()? .concat(); match func { - AggFunc::Sum => Ok(values.iter().sum()), + AggFunc::Sum => Ok(exact_sum(&values)), AggFunc::Count => Ok(values.len() as f64), AggFunc::Avg | AggFunc::Min | AggFunc::Max => { apply_agg(&values, func).ok_or_else(|| "empty".to_string()) diff --git a/crates/improvise-incremental/Cargo.toml b/crates/improvise-incremental/Cargo.toml index 2eed040a..20d49e06 100644 --- a/crates/improvise-incremental/Cargo.toml +++ b/crates/improvise-incremental/Cargo.toml @@ -9,6 +9,7 @@ description = "The seam between improvise-core and the incremental circuit: the # `zset` edge is the cell delta of step 3 (improvise-5r8c). [dependencies] improvise-core = { path = "../improvise-core" } +numtower = { path = "../numtower" } zset = { path = "../zset" } [dev-dependencies] diff --git a/crates/improvise-incremental/src/group.rs b/crates/improvise-incremental/src/group.rs index c4d045aa..0ca19195 100644 --- a/crates/improvise-incremental/src/group.rs +++ b/crates/improvise-incremental/src/group.rs @@ -1,18 +1,20 @@ -//! One group's maintained state and how it is read (ADR 009 clause 4). -//! Reasoning: `context/code/improvise-incremental.md`, step 4. +//! One group's maintained state and how it is read (ADR 009 clauses 4 and 8). +//! Reasoning: `context/code/improvise-incremental.md`, steps 4 and 6. use crate::DeltaValue; use improvise_core::formula::ast::AggFunc; use improvise_core::model::cell::CellValue; +use numtower::FloatSum; use std::collections::BTreeMap; -/// A group's cells: how many, how many are numbers, and a multiset of their values. -/// An update touches one map entry, so it costs O(log d) in the distinct values. +/// A group's cells: how many, how many are numbers, a multiset of their values and their exact +/// sum. An update touches one map entry, so it costs O(log d) in the distinct values. #[derive(Debug, Clone, Default)] pub struct Group { cells: i64, numbers: i64, values: BTreeMap, + sum: FloatSum, } impl Group { @@ -29,6 +31,12 @@ impl Group { }; } + /// Adds `weight` copies of the number `x` to the sum, signed zeros as stored: the multiset + /// calls the two zeros equal, the sum must not. + pub fn add_number(&mut self, x: f64, weight: i64) { + self.sum.add(x, weight); + } + pub fn cells(&self) -> i64 { self.cells } @@ -42,6 +50,9 @@ impl Group { let mut group = Group::default(); for value in values { group.apply(&DeltaValue::new(value), 1); + if let CellValue::Number(x) = value { + group.add_number(*x, 1); + } } group } @@ -64,29 +75,31 @@ fn non_nan(value: &DeltaValue) -> Option { } } -/// The aggregates a group answers without looking at order: step 4's. +/// The aggregates a group answers, none of which looks at order. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum OrderFree { +pub enum Reduce { Count, Min, Max, + Sum, + Avg, } -impl OrderFree { - /// `Sum` and `Avg` depend on the order eager adds in, so they are not here. - pub fn of(func: &AggFunc) -> Option { +impl Reduce { + pub fn of(func: &AggFunc) -> Self { match func { - AggFunc::Count => Some(OrderFree::Count), - AggFunc::Min => Some(OrderFree::Min), - AggFunc::Max => Some(OrderFree::Max), - AggFunc::Sum | AggFunc::Avg => None, + AggFunc::Count => Reduce::Count, + AggFunc::Min => Reduce::Min, + AggFunc::Max => Reduce::Max, + AggFunc::Sum => Reduce::Sum, + AggFunc::Avg => Reduce::Avg, } } } /// What eager's `aggregate_cell_values` returns for the union of `groups`: numbers reduce, /// anything else is unanimous or `Error("mixed")`, and no cells is absent. -pub fn fold_groups(groups: &[&Group], func: OrderFree) -> Option { +pub fn fold_groups(groups: &[&Group], func: Reduce) -> Option { let cells: i64 = groups.iter().map(|g| g.cells).sum(); let numbers: i64 = groups.iter().map(|g| g.numbers).sum(); match (cells, numbers == cells) { @@ -97,20 +110,29 @@ pub fn fold_groups(groups: &[&Group], func: OrderFree) -> Option { } /// `Min` and `Max` skip NaN, and are infinite when nothing else is left, as eager's folds are. -fn reduce(groups: &[&Group], func: OrderFree, cells: i64) -> f64 { +fn reduce(groups: &[&Group], func: Reduce, cells: i64) -> f64 { match func { - OrderFree::Count => cells as f64, - OrderFree::Min => { + Reduce::Count => cells as f64, + Reduce::Sum => total(groups).value(), + Reduce::Avg => total(groups).value() / cells as f64, + Reduce::Min => { let smallest = groups.iter().filter_map(|g| g.smallest()); smallest.fold(f64::INFINITY, f64::min) } - OrderFree::Max => { + Reduce::Max => { let largest = groups.iter().filter_map(|g| g.largest()); largest.fold(f64::NEG_INFINITY, f64::max) } } } +/// The groups' sums, absorbed into one: exact, so the same in any order and for any grouping. +fn total(groups: &[&Group]) -> FloatSum { + let mut sum = FloatSum::new(); + groups.iter().for_each(|g| sum.absorb(&g.sum)); + sum +} + fn unanimous(groups: &[&Group]) -> CellValue { let mut distinct = groups.iter().flat_map(|g| g.values.keys()); let first = distinct.next(); @@ -138,7 +160,7 @@ mod tests { g.apply(&DeltaValue::new(&num(1.0)), -1); assert_eq!(g.cells(), 1); assert_eq!( - fold_groups(&[&g], OrderFree::Min), + fold_groups(&[&g], Reduce::Min), Some(num(2.0)), "the retracted minimum is gone" ); @@ -150,45 +172,45 @@ mod tests { g.apply(&DeltaValue::new(&num(1.0)), -1); assert!(g.is_empty()); assert!(g.values.is_empty(), "no zero-weight entry is left behind"); - assert_eq!(fold_groups(&[&g], OrderFree::Count), None); + assert_eq!(fold_groups(&[&g], Reduce::Count), None); } #[test] fn min_and_max_skip_nan_and_are_infinite_over_nothing_else() { let some = group(&[num(f64::NAN), num(3.0), num(-1.0)]); - assert_eq!(fold_groups(&[&some], OrderFree::Min), Some(num(-1.0))); - assert_eq!(fold_groups(&[&some], OrderFree::Max), Some(num(3.0))); + assert_eq!(fold_groups(&[&some], Reduce::Min), Some(num(-1.0))); + assert_eq!(fold_groups(&[&some], Reduce::Max), Some(num(3.0))); let only_nan = group(&[num(f64::NAN), num(-f64::NAN)]); assert_eq!( - fold_groups(&[&only_nan], OrderFree::Min), + fold_groups(&[&only_nan], Reduce::Min), Some(num(f64::INFINITY)) ); assert_eq!( - fold_groups(&[&only_nan], OrderFree::Max), + fold_groups(&[&only_nan], Reduce::Max), Some(num(f64::NEG_INFINITY)) ); - assert_eq!(fold_groups(&[&only_nan], OrderFree::Count), Some(num(2.0))); + assert_eq!(fold_groups(&[&only_nan], Reduce::Count), Some(num(2.0))); } #[test] fn a_fold_over_several_groups_is_the_fold_of_their_union() { let (a, b) = (group(&[num(5.0), num(9.0)]), group(&[num(2.0)])); - assert_eq!(fold_groups(&[&a, &b], OrderFree::Min), Some(num(2.0))); - assert_eq!(fold_groups(&[&a, &b], OrderFree::Max), Some(num(9.0))); - assert_eq!(fold_groups(&[&a, &b], OrderFree::Count), Some(num(3.0))); + assert_eq!(fold_groups(&[&a, &b], Reduce::Min), Some(num(2.0))); + assert_eq!(fold_groups(&[&a, &b], Reduce::Max), Some(num(9.0))); + assert_eq!(fold_groups(&[&a, &b], Reduce::Count), Some(num(3.0))); } #[test] fn text_is_unanimous_or_mixed_across_groups() { let text = |s: &str| CellValue::Text(s.into()); let (a, b) = (group(&[text("x"), text("x")]), group(&[text("x")])); - assert_eq!(fold_groups(&[&a, &b], OrderFree::Min), Some(text("x"))); + assert_eq!(fold_groups(&[&a, &b], Reduce::Min), Some(text("x"))); let c = group(&[text("y")]); let mixed = Some(CellValue::Error("mixed".into())); - assert_eq!(fold_groups(&[&a, &c], OrderFree::Min), mixed); + assert_eq!(fold_groups(&[&a, &c], Reduce::Min), mixed); let with_number = group(&[text("x"), num(1.0)]); - assert_eq!(fold_groups(&[&with_number], OrderFree::Count), mixed); + assert_eq!(fold_groups(&[&with_number], Reduce::Count), mixed); let nan_among_text = group(&[num(f64::NAN), text("x")]); - assert_eq!(fold_groups(&[&nan_among_text], OrderFree::Count), mixed); + assert_eq!(fold_groups(&[&nan_among_text], Reduce::Count), mixed); } } diff --git a/crates/improvise-incremental/src/incremental.rs b/crates/improvise-incremental/src/incremental.rs index 990eaabd..b271a4ac 100644 --- a/crates/improvise-incremental/src/incremental.rs +++ b/crates/improvise-incremental/src/incremental.rs @@ -3,7 +3,7 @@ use crate::fold::Expansion; use crate::formulas::{PASSES, Stems}; -use crate::group::{Group, OrderFree, fold_groups}; +use crate::group::{Group, Reduce, fold_groups}; use crate::shapes::{Meter, Shapes}; use crate::{Derived, Edit, Maintained, Reading, cell_delta}; use improvise_core::formula::ast::AggFunc; @@ -86,14 +86,13 @@ impl Incremental { covers: Covers, over: &[String], ) -> Option { - let owned = self.model.formula_for(key).is_some(); - let func = OrderFree::of(self.func_for(key)).filter(|_| !owned); - let Some(func) = func else { + if self.model.formula_for(key).is_some() { return match over.is_empty() { true => self.model.evaluate_covering(key, agg, covers), false => self.model.evaluate_spanning(key, over, agg, covers), }; - }; + } + let func = Reduce::of(self.func_for(key)); let expansion = Expansion::of(&self.model, key, covers, over); match (expansion.spans.is_empty(), agg) { (true, Aggregation::Exact) => self.model.data.get(key).cloned(), @@ -107,7 +106,7 @@ impl Incremental { } } - fn fold(&self, keys: &[CellKey], func: OrderFree) -> Option { + fn fold(&self, keys: &[CellKey], func: Reduce) -> Option { self.shapes .read(&self.model.data, keys, |groups| fold_groups(groups, func)) } @@ -350,7 +349,7 @@ impl Maintained for Incremental { fn apply(&mut self, edit: &Edit) { edit.apply(&mut self.model); let changes = self.model.data.drain_changes(); - self.shapes.apply(&cell_delta(&changes)); + self.shapes.apply(&cell_delta(&changes), &changes); self.refresh_formulas(edit, &changes); } } diff --git a/crates/improvise-incremental/src/shapes.rs b/crates/improvise-incremental/src/shapes.rs index 8d8efe65..1f1e2d9c 100644 --- a/crates/improvise-incremental/src/shapes.rs +++ b/crates/improvise-incremental/src/shapes.rs @@ -1,9 +1,9 @@ //! The maintained groups, one map per set of named categories (ADR 009 clause 7). -//! Reasoning: `context/code/improvise-incremental.md`, step 4. +//! Reasoning: `context/code/improvise-incremental.md`, steps 4 and 6. use crate::group::Group; use crate::{CellDelta, DeltaValue}; -use improvise_core::model::cell::{CellKey, DataStore}; +use improvise_core::model::cell::{CellChange, CellKey, CellValue, DataStore}; use std::cell::{Cell, RefCell}; use std::collections::BTreeMap; @@ -54,22 +54,73 @@ fn project(key: &CellKey, cats: &[String]) -> Option { coords.map(CellKey::new) } +/// A change that left the same bits behind moves no sum. +fn same_bits(change: &CellChange) -> bool { + match (&change.old, &change.new) { + (Some(CellValue::Number(a)), Some(CellValue::Number(b))) => a.to_bits() == b.to_bits(), + (a, b) => a == b, + } +} + impl Shape { - fn apply(&mut self, cats: &[String], delta: &CellDelta) -> u64 { + /// The canonical delta moves counts and multisets; the raw changes move sums, which tell the + /// two zeros apart. Returns the group updates the delta caused. + fn apply(&mut self, cats: &[String], delta: &CellDelta, changes: &[CellChange]) -> u64 { let mut updates = 0; + let mut touched = Vec::new(); for ((key, value), weight) in delta.iter() { let Some(at) = project(key, cats) else { continue; }; - let group = self.groups.entry(at.clone()).or_default(); - group.apply(value, weight); + self.groups + .entry(at.clone()) + .or_default() + .apply(value, weight); + touched.push(at); updates += 1; - if group.is_empty() { + } + for change in changes.iter().filter(|c| !same_bits(c)) { + let Some(at) = project(&change.key, cats) else { + continue; + }; + let Some(group) = self.groups.get_mut(&at) else { + continue; + }; + if let Some(CellValue::Number(old)) = &change.old { + group.add_number(*old, -1); + } + if let Some(CellValue::Number(new)) = &change.new { + group.add_number(*new, 1); + } + touched.push(at); + } + for at in touched { + if self.groups.get(&at).is_some_and(Group::is_empty) { self.groups.remove(&at); } } updates } + + /// The shape of `store`'s cells, one pass. + fn of(store: &DataStore, cats: &[String]) -> (Shape, u64) { + let mut shape = Shape { + groups: BTreeMap::new(), + }; + let mut visited = 0; + for (key, value) in store.iter_cells() { + visited += 1; + let Some(at) = project(&key, cats) else { + continue; + }; + let group = shape.groups.entry(at).or_default(); + group.apply(&DeltaValue::new(value), 1); + if let CellValue::Number(x) = value { + group.add_number(*x, 1); + } + } + (shape, visited) + } } /// Every shape a read has asked for. A shape is built from the store on its first read (a @@ -95,13 +146,13 @@ impl Shapes { self.built.borrow().len() } - /// Feeds `delta` to every shape built so far. - pub fn apply(&mut self, delta: &CellDelta) { + /// Feeds a batch of changes, and the delta they make, to every shape built so far. + pub fn apply(&mut self, delta: &CellDelta, changes: &[CellChange]) { let updates: u64 = self .built .get_mut() .iter_mut() - .map(|(cats, shape)| shape.apply(cats, delta)) + .map(|(cats, shape)| shape.apply(cats, delta, changes)) .sum(); self.bump(|m| m.updates += updates); } @@ -120,15 +171,7 @@ impl Shapes { let cats: Vec = first.0.iter().map(|(c, _)| c.clone()).collect(); let mut built = self.built.borrow_mut(); let shape = built.entry(cats.clone()).or_insert_with(|| { - let mut shape = Shape { - groups: BTreeMap::new(), - }; - let cells = store.iter_cells(); - let all: CellDelta = cells - .map(|(k, v)| ((k, DeltaValue::new(v)), 1)) - .collect::(); - let visited = all.len() as u64; - shape.apply(&cats, &all); + let (shape, visited) = Shape::of(store, &cats); self.bump(|m| { m.builds += 1; m.built_cells += visited; diff --git a/crates/improvise-incremental/tests/exact.rs b/crates/improvise-incremental/tests/exact.rs new file mode 100644 index 00000000..0c343313 --- /dev/null +++ b/crates/improvise-incremental/tests/exact.rs @@ -0,0 +1,139 @@ +//! `Sum` and `Avg` in both backends: exact, order-free, and signed as IEEE signs the same addends +//! (ADR 009 clause 8). Evidence: `context/test-evidence/improvise-incremental.md`. + +use improvise_core::formula::ast::AggFunc; +use improvise_core::model::Model; +use improvise_core::model::cell::{CellKey, CellValue}; +use improvise_incremental::{Derived, Eager, Edit, Incremental, Maintained}; +use numtower::FloatSum; + +fn key(i: usize) -> CellKey { + CellKey::new(vec![ + ("Region".into(), format!("r{i}")), + ("_Measure".into(), "Revenue".into()), + ]) +} + +fn the_measure() -> CellKey { + CellKey::new(vec![("_Measure".into(), "Revenue".into())]) +} + +fn model_of(values: &[f64], func: AggFunc) -> Model { + let mut model = Model::new("sums"); + model.add_category("Region").unwrap(); + model.category_mut("_Measure").unwrap().add_item("Revenue"); + model.measure_agg.insert("Revenue".into(), func); + for (i, v) in values.iter().enumerate() { + model + .category_mut("Region") + .unwrap() + .add_item(format!("r{i}")); + model.set_cell(key(i), CellValue::Number(*v)); + } + model +} + +fn exact(values: &[f64]) -> f64 { + let mut sum = FloatSum::new(); + values.iter().for_each(|v| sum.add(*v, 1)); + sum.value() +} + +/// Both backends' sum of `values` under `func`, as bits. +fn both(values: &[f64], func: AggFunc) -> (Option, Option) { + let model = model_of(values, func); + let bits = |v: Option| v.and_then(|v| v.as_f64()).map(f64::to_bits); + let eager = Eager::new(model.clone(), vec![]); + let incremental = Incremental::new(model, vec![]); + let at = the_measure(); + (bits(eager.raw_fold(&at)), bits(incremental.raw_fold(&at))) +} + +#[test] +fn sums_are_exact_whatever_the_order_the_cells_were_added_in() { + let values = [1e16, 1.0, -1e16]; + let orders = [ + [0, 1, 2], + [0, 2, 1], + [1, 0, 2], + [1, 2, 0], + [2, 0, 1], + [2, 1, 0], + ]; + for order in orders { + let added: Vec = order.iter().map(|&i| values[i]).collect(); + let want = Some(1.0f64.to_bits()); + assert_eq!(both(&added, AggFunc::Sum), (want, want), "{added:?}"); + } +} + +#[test] +fn an_average_is_the_exact_sum_over_the_count() { + let values = [0.1, 0.2, 0.3, 1e16, -1e16]; + let want = Some((exact(&values) / values.len() as f64).to_bits()); + assert_eq!(both(&values, AggFunc::Avg), (want, want)); +} + +#[test] +fn a_zero_total_has_the_sign_ieee_gives_the_same_addends() { + let cases: [(&[f64], f64); 4] = [ + (&[-0.0, -0.0], -0.0), + (&[-0.0, 0.0], 0.0), + (&[1.0, -1.0], 0.0), + (&[-0.0, 3.0, -3.0], 0.0), + ]; + for (values, sign) in cases { + let want = Some(sign.to_bits()); + assert_eq!(both(values, AggFunc::Sum), (want, want), "{values:?}"); + } +} + +#[test] +fn overwriting_zero_with_negative_zero_moves_the_sign_of_a_sum() { + let model = model_of(&[0.0, 0.0], AggFunc::Sum); + let mut eager = Eager::new(model.clone(), vec![]); + let mut incremental = Incremental::new(model, vec![]); + let at = the_measure(); + let bits = |b: &dyn Derived| b.raw_fold(&at).and_then(|v| v.as_f64()).map(f64::to_bits); + assert_eq!(bits(&incremental), Some(0.0f64.to_bits())); + for i in 0..2 { + let edit = Edit::SetCell(key(i), CellValue::Number(-0.0)); + eager.apply(&edit); + incremental.apply(&edit); + } + let negative = Some((-0.0f64).to_bits()); + assert_eq!((bits(&eager), bits(&incremental)), (negative, negative)); + let back = Edit::SetCell(key(1), CellValue::Number(0.0)); + eager.apply(&back); + incremental.apply(&back); + let positive = Some(0.0f64.to_bits()); + assert_eq!((bits(&eager), bits(&incremental)), (positive, positive)); +} + +#[test] +fn a_million_overwrites_in_a_thousand_cell_group_do_not_drift() { + let values: Vec = (0..1000).map(|i| 0.1 * f64::from(i % 7 + 1)).collect(); + let model = model_of(&values, AggFunc::Sum); + let mut eager = Eager::new(model.clone(), vec![]); + let mut incremental = Incremental::new(model, vec![]); + let at = the_measure(); + let mut seed = 7u64; + for step in 0..20_000 { + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + let (i, wide) = ((seed >> 33) as usize % 1000, (seed >> 20) % 3); + let v = [0.1, 1e12 + 0.3, -7.77e-3][wide as usize] * f64::from((step % 5) as u8 + 1); + let edit = Edit::SetCell(key(i), CellValue::Number(v)); + eager.apply(&edit); + incremental.apply(&edit); + } + let bits = |b: &dyn Derived| b.raw_fold(&at).and_then(|v| v.as_f64()).map(f64::to_bits); + assert_eq!(bits(&eager), bits(&incremental)); + let fresh = Incremental::new(eager.model().clone(), vec![]); + assert_eq!( + bits(&fresh), + bits(&incremental), + "and equals a sum built afresh" + ); +} diff --git a/crates/improvise-incremental/tests/measure.rs b/crates/improvise-incremental/tests/measure.rs index 0c28c50d..10908f88 100644 --- a/crates/improvise-incremental/tests/measure.rs +++ b/crates/improvise-incremental/tests/measure.rs @@ -92,8 +92,8 @@ fn a_group_read_comes_from_a_maintained_group() { } /// The operations one overwrite costs once `shapes` category sets are maintained. -fn overwrite_cost(n: usize, shapes: usize) -> (Meter, usize) { - let mut backend = Incremental::new(table(n, AggFunc::Max), vec![]); +fn overwrite_cost(n: usize, shapes: usize, func: AggFunc) -> (Meter, usize) { + let mut backend = Incremental::new(table(n, func), vec![]); let partials = [region_partial(), product_partial()]; for partial in partials.iter().take(shapes) { backend.raw_fold(partial); @@ -101,27 +101,54 @@ fn overwrite_cost(n: usize, shapes: usize) -> (Meter, usize) { let before = backend.meter(); backend.apply(&Edit::SetCell(cell(1, 2, 3), CellValue::Number(2000.0))); let cost = since(before, backend.meter()); - let seen = backend.raw_fold(®ion_partial()); - assert_eq!(seen, Some(CellValue::Number(2000.0))); + assert!(backend.raw_fold(®ion_partial()).is_some()); (cost, backend.shape_count()) } #[test] fn an_overwrite_costs_two_updates_per_shape_whatever_the_table_size() { - for shapes in [1, 2] { - let (small, built) = overwrite_cost(6, shapes); - let (large, _) = overwrite_cost(12, shapes); - assert!(built >= shapes); - assert_eq!( - small.updates, - 2 * shapes as u64, - "retract and insert, per shape" - ); - assert_eq!(small, large, "216 cells and 864 cells cost the same"); - assert_eq!(large.builds, 0); + let funcs = [ + AggFunc::Max, + AggFunc::Min, + AggFunc::Count, + AggFunc::Sum, + AggFunc::Avg, + ]; + for func in funcs { + for shapes in [1, 2] { + let (small, built) = overwrite_cost(6, shapes, func.clone()); + let (large, _) = overwrite_cost(12, shapes, func.clone()); + assert!(built >= shapes); + let updates = 2 * shapes as u64; + assert_eq!( + small.updates, updates, + "{func:?}: retract and insert, per shape" + ); + assert_eq!( + small, large, + "{func:?}: 216 cells and 864 cells cost the same" + ); + assert_eq!(large.builds, 0); + } } } +#[test] +fn a_sum_read_after_a_write_comes_from_the_delta_and_equals_a_fresh_sum() { + let mut backend = Incremental::new(table(6, AggFunc::Sum), vec![]); + backend.raw_fold(®ion_partial()); + backend.apply(&Edit::SetCell(cell(1, 0, 0), CellValue::Number(0.1))); + let built = backend.meter().builds; + let got = backend.raw_fold(®ion_partial()); + assert_eq!( + backend.meter().builds, + built, + "no second pass over the store" + ); + let fresh = Incremental::new(backend.model().clone(), vec![]); + assert_eq!(got, fresh.raw_fold(®ion_partial())); +} + #[test] fn a_read_visits_one_group_where_eager_visits_every_cell_it_covers() { for n in [6, 12] { diff --git a/crates/improvise-incremental/tests/random.rs b/crates/improvise-incremental/tests/random.rs index 57dded9f..fd171224 100644 --- a/crates/improvise-incremental/tests/random.rs +++ b/crates/improvise-incremental/tests/random.rs @@ -20,6 +20,7 @@ fn pool() -> impl Strategy { Just(CellValue::Number(f64::NAN)), Just(CellValue::Number(-f64::NAN)), Just(CellValue::Number(f64::INFINITY)), + prop::sample::select(vec![0.1, 1e16, -1e16, f64::MAX, 5e-324]).prop_map(CellValue::Number), prop::sample::select(vec!["n", "x"]).prop_map(|t| CellValue::Text(t.into())), Just(CellValue::Error("e".into())), ] diff --git a/crates/improvise-incremental/tests/support/mod.rs b/crates/improvise-incremental/tests/support/mod.rs index a21a31bc..2c7e0508 100644 --- a/crates/improvise-incremental/tests/support/mod.rs +++ b/crates/improvise-incremental/tests/support/mod.rs @@ -338,6 +338,7 @@ fn any_value() -> impl Strategy { (-2i32..4).prop_map(|n| CellValue::Number(f64::from(n))), Just(CellValue::Number(-0.0)), Just(CellValue::Number(f64::NAN)), + prop::sample::select(vec![0.1, 1e16, -1e16]).prop_map(CellValue::Number), Just(CellValue::Text("t".into())), ] } diff --git a/crates/numtower/src/lib.rs b/crates/numtower/src/lib.rs index 458d3e9f..cd8374b3 100644 --- a/crates/numtower/src/lib.rs +++ b/crates/numtower/src/lib.rs @@ -14,7 +14,7 @@ pub use arith::{CEILING_BITS, DivisionByZero, Interim}; pub use lang::{Typing, is_number, parse_number, shortest_round_trip}; pub use num::{Num, NumRef}; pub use round::Rounding; -pub use sum::{ExactSum, K, NumSum}; +pub use sum::{ExactSum, FloatSum, K, NumSum}; pub use num_rational::Ratio; pub use rust_decimal::Decimal; diff --git a/crates/numtower/src/sum.rs b/crates/numtower/src/sum.rs index 366edbb5..6e225780 100644 --- a/crates/numtower/src/sum.rs +++ b/crates/numtower/src/sum.rs @@ -144,6 +144,45 @@ impl ExactSum { } } +/// An `ExactSum` that also gives a zero total the sign IEEE gives the same addends (clause 8): +/// `-0.0` only if every addend is `-0.0`, else `+0.0`. An empty sum is `-0.0`, as `Sum for f64` is. +#[derive(Clone, Debug, Default)] +pub struct FloatSum { + exact: ExactSum, + addends: i128, + negative_zeros: i128, +} + +impl FloatSum { + pub fn new() -> FloatSum { + FloatSum::default() + } + + /// Adds `x` with a Z-set `weight`, negative to retract. + pub fn add(&mut self, x: f64, weight: i64) { + self.exact.add(x, weight); + self.addends += i128::from(weight); + if x == 0.0 && x.is_sign_negative() { + self.negative_zeros += i128::from(weight); + } + } + + /// Adds another accumulator's contents. + pub fn absorb(&mut self, o: &FloatSum) { + self.exact.absorb(&o.exact); + self.addends += o.addends; + self.negative_zeros += o.negative_zeros; + } + + /// The exact sum rounded to the nearest f64, ties to even. + pub fn value(&self) -> f64 { + match self.exact.value() { + v if v == 0.0 && self.negative_zeros == self.addends => -0.0, + v => v, + } + } +} + /// Carry each limb into the next, leaving every limb but the top in [0, 2^32). fn propagate(limbs: &mut [i64; LIMBS]) { for k in 0..LIMBS - 1 { diff --git a/crates/numtower/tests/float_sum.rs b/crates/numtower/tests/float_sum.rs new file mode 100644 index 00000000..4f32cb8f --- /dev/null +++ b/crates/numtower/tests/float_sum.rs @@ -0,0 +1,78 @@ +//! `FloatSum`: ADR 009 clause 8's sign of a zero total, against IEEE addition. +//! Evidence: `context/test-evidence/numtower.md`. + +use numtower::FloatSum; +use proptest::prelude::*; + +/// Both zeros and small integers: totals that cancel, and every sum below 2^53 is exact. +fn addend() -> impl Strategy { + prop_oneof![ + Just(0.0), + Just(-0.0), + (-3i32..=3).prop_map(f64::from), + prop::sample::select(vec![0.5, -0.5]), + ] +} + +fn sum_of(values: &[f64]) -> FloatSum { + let mut sum = FloatSum::new(); + for v in values { + sum.add(*v, 1); + } + sum +} + +/// What adding the same addends in `f64` gives, starting from `-0.0` as `Sum for f64` does. +fn ieee(values: &[f64]) -> f64 { + values.iter().sum() +} + +fn same_bits(a: f64, b: f64) -> bool { + a.to_bits() == b.to_bits() +} + +#[test] +fn the_sign_of_a_zero_total_follows_ieee_for_the_cases_that_decide_it() { + assert!(same_bits(sum_of(&[-0.0, -0.0]).value(), -0.0)); + assert!(same_bits(sum_of(&[-0.0, 0.0]).value(), 0.0)); + assert!(same_bits(sum_of(&[0.0, 0.0]).value(), 0.0)); + assert!(same_bits(sum_of(&[1.0, -1.0]).value(), 0.0)); + assert!(same_bits(sum_of(&[-0.0, 1.0, -1.0]).value(), 0.0)); + assert!(same_bits(sum_of(&[]).value(), -0.0), "as Sum for f64 is"); +} + +proptest! { + #[test] + fn a_sum_has_the_bits_ieee_gives_the_same_addends( + values in prop::collection::vec(addend(), 0..12), + ) { + let (got, want) = (sum_of(&values).value(), ieee(&values)); + prop_assert!(same_bits(got, want), "{got:?} for {want:?} over {values:?}"); + } + + #[test] + fn retracting_addends_leaves_the_sum_of_the_rest( + values in prop::collection::vec(addend(), 1..12), + keep in 0usize..12, + ) { + let keep = keep.min(values.len()); + let mut sum = sum_of(&values); + for v in &values[keep..] { + sum.add(*v, -1); + } + let (got, want) = (sum.value(), ieee(&values[..keep])); + prop_assert!(same_bits(got, want), "{got:?} for {want:?} over {values:?} keeping {keep}"); + } + + #[test] + fn absorbing_two_sums_is_adding_all_the_addends( + values in prop::collection::vec(addend(), 0..12), + cut in 0usize..12, + ) { + let cut = cut.min(values.len()); + let mut left = sum_of(&values[..cut]); + left.absorb(&sum_of(&values[cut..])); + let (got, want) = (left.value(), sum_of(&values).value()); + prop_assert!(same_bits(got, want), "{got:?} for {want:?} over {values:?} cut at {cut}"); + } +} diff --git a/harness/opfs-contract/Cargo.lock b/harness/opfs-contract/Cargo.lock index 704469e7..2eb84d59 100644 --- a/harness/opfs-contract/Cargo.lock +++ b/harness/opfs-contract/Cargo.lock @@ -23,18 +23,54 @@ version = "1.0.104" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "330a5ed07fa54e4702c9d6c4174f74427fc0ef6e214bbd677ae50a5099946470" +[[package]] +name = "arrayvec" +version = "0.7.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3fb67a6e08acf24fdeccbac2cb6ac4305825bd1f117462e0e6f2f193345ad56" + [[package]] name = "autocfg" version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" +[[package]] +name = "borsh" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "553c5d846a6ba5150c65e3b1b8ec073bcf1abc20f9b7220de384a4443ea4e20a" +dependencies = [ + "borsh-derive", + "bytes", + "cfg_aliases", +] + +[[package]] +name = "borsh-derive" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "12cdfe656708a01f89b451a7d36466e6fe6c414de0aa18fc54f864f6f9ca9f56" +dependencies = [ + "once_cell", + "proc-macro-crate", + "proc-macro2", + "quote", + "syn 3.0.6", +] + [[package]] name = "bumpalo" version = "3.20.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" +[[package]] +name = "bytes" +version = "1.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc652a48c352aef3ea3aed32080501cf3ef6ed5da78602a020c991775b0aff04" + [[package]] name = "cc" version = "1.4.7" @@ -51,6 +87,12 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4e7648175b45a9a48536d676f68d918270699102aa8dab5496df06904c914600" +[[package]] +name = "cfg_aliases" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f079e83a288787bcd14a6aea84cee5c87a67c5a3e660c30f557a3d24761b3527" + [[package]] name = "chrono" version = "0.4.45" @@ -148,6 +190,29 @@ dependencies = [ "slab", ] +[[package]] +name = "getrandom" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ff2abc00be7fca6ebc474524697ae276ad847ad0a6b3faa4bcb027e9a4614ad0" +dependencies = [ + "cfg-if", + "libc", + "wasi", +] + +[[package]] +name = "getrandom" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "899def5c37c4fd7b2664648c28120ecec138e4d395b459e5ca34f9cce2dd77fd" +dependencies = [ + "cfg-if", + "libc", + "r-efi", + "wasip2", +] + [[package]] name = "hashbrown" version = "0.17.1" @@ -185,6 +250,7 @@ dependencies = [ "anyhow", "improvise-formula", "indexmap", + "numtower", "serde", "siphasher", "unicase", @@ -276,6 +342,35 @@ dependencies = [ "simd-adler32", ] +[[package]] +name = "num-bigint" +version = "0.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c89e69e7e0f03bea5ef08013795c25018e101932225a656383bd384495ecc367" +dependencies = [ + "num-integer", + "num-traits", +] + +[[package]] +name = "num-integer" +version = "0.1.47" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7ce2d95d4b3734dc35aa2f45e1aa22cd416814592a4f9d9205e11affd5b8e10b" +dependencies = [ + "num-traits", +] + +[[package]] +name = "num-rational" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" +dependencies = [ + "num-integer", + "num-traits", +] + [[package]] name = "num-traits" version = "0.2.19" @@ -285,6 +380,17 @@ dependencies = [ "autocfg", ] +[[package]] +name = "numtower" +version = "0.1.0" +dependencies = [ + "num-bigint", + "num-integer", + "num-rational", + "num-traits", + "rust_decimal", +] + [[package]] name = "once_cell" version = "1.21.4" @@ -355,6 +461,24 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" +[[package]] +name = "ppv-lite86" +version = "0.2.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85eae3c4ed2f50dcfe72643da4befc30deadb458a9b590d720cde2f2b1e97da9" +dependencies = [ + "zerocopy", +] + +[[package]] +name = "proc-macro-crate" +version = "3.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e67ba7e9b2b56446f1d419b1d807906278ffa1a658a8a5d8a39dcb1f5a78614f" +dependencies = [ + "toml_edit", +] + [[package]] name = "proc-macro2" version = "1.0.107" @@ -382,6 +506,88 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "r-efi" +version = "5.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f" + +[[package]] +name = "rand" +version = "0.8.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e058c7de0b26af77780c769414d6257830bb240f3c38477dbc2c16e5f54d6d4c" +dependencies = [ + "libc", + "rand_chacha 0.3.1", + "rand_core 0.6.4", +] + +[[package]] +name = "rand" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9ef1d0d795eb7d84685bca4f72f3649f064e6641543d3a8c415898726a57b41" +dependencies = [ + "rand_chacha 0.9.0", + "rand_core 0.9.5", +] + +[[package]] +name = "rand_chacha" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" +dependencies = [ + "ppv-lite86", + "rand_core 0.6.4", +] + +[[package]] +name = "rand_chacha" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3022b5f1df60f26e1ffddd6c66e8aa15de382ae63b3a0c1bfc0e4d3e3f325cb" +dependencies = [ + "ppv-lite86", + "rand_core 0.9.5", +] + +[[package]] +name = "rand_core" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" +dependencies = [ + "getrandom 0.2.17", +] + +[[package]] +name = "rand_core" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76afc826de14238e6e8c374ddcc1fa19e374fd8dd986b0d2af0d02377261d83c" +dependencies = [ + "getrandom 0.3.4", +] + +[[package]] +name = "rust_decimal" +version = "1.43.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7653272e75dcac41dc199fbea6f5797633994fafd339943c06c9af16bf29cd3a" +dependencies = [ + "arrayvec", + "borsh", + "bytes", + "num-traits", + "rand 0.8.8", + "rand 0.9.5", + "serde", + "serde_json", + "wasm-bindgen", +] + [[package]] name = "rustversion" version = "1.0.23" @@ -505,6 +711,36 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "toml_datetime" +version = "1.1.2+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b86d767906c6c42421dcba507eb9d203e779497710a47782a224bb871653053" +dependencies = [ + "serde_core", +] + +[[package]] +name = "toml_edit" +version = "0.25.17+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3641d5bbb5349a79e1020a242d251efbc546ad8048d133958323ce9c40a9c9c" +dependencies = [ + "indexmap", + "toml_datetime", + "toml_parser", + "winnow", +] + +[[package]] +name = "toml_parser" +version = "1.1.5+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baa693a8032d7e1cada7d0041e96126df243179ff061456783ac7f12bda4744c" +dependencies = [ + "winnow", +] + [[package]] name = "ucd-trie" version = "0.1.7" @@ -523,6 +759,21 @@ version = "1.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d245f478577f809a851594d02313b640fb437e0bb33866753cff937863096954" +[[package]] +name = "wasi" +version = "0.11.1+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" + +[[package]] +name = "wasip2" +version = "1.0.4+wasi-0.2.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b67efb37e106e55ce722a510d6b5f9c17f083e5fc79afc2badeb12cc313d9487" +dependencies = [ + "wit-bindgen", +] + [[package]] name = "wasm-bindgen" version = "0.2.129" @@ -657,6 +908,41 @@ dependencies = [ "windows-link", ] +[[package]] +name = "winnow" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23b97319f7b8343df12cc98938e5c3eb436064524c8d2b4e30a1d3a36eecdf81" +dependencies = [ + "memchr", +] + +[[package]] +name = "wit-bindgen" +version = "0.57.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ebf944e87a7c253233ad6766e082e3cd714b5d03812acc24c318f549614536e" + +[[package]] +name = "zerocopy" +version = "0.8.62" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "86502bf56ac7c77571a32e2647bb2a15894565e981fb2a48d7bde2d91c965a9d" +dependencies = [ + "zerocopy-derive", +] + +[[package]] +name = "zerocopy-derive" +version = "0.8.62" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5457206954b06561e2608c7e19cf58b1926586d999c246eebe4502f7e2039d1a" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.119", +] + [[package]] name = "zlib-rs" version = "0.6.8"