diff --git a/crates/improvise-core/src/model/cell.rs b/crates/improvise-core/src/model/cell.rs index ab23eb70..2a913b2e 100644 --- a/crates/improvise-core/src/model/cell.rs +++ b/crates/improvise-core/src/model/cell.rs @@ -7,7 +7,7 @@ use super::symbol::{Symbol, SymbolTable}; /// A cell key: (category, item) pairs, always sorted by category name. The /// constructors are the only way to build one — context/code/core.md §1. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] pub struct CellKey(pub Vec<(String, String)>); impl CellKey { @@ -194,6 +194,15 @@ impl RecordEntry { } } +/// One cell's move in the store: `old` is `None` for a new cell, `new` is +/// `None` for a removal. ADR 009 clause 3; `context/code/improvise-incremental.md`. +#[derive(Debug, Clone, PartialEq)] +pub struct CellChange { + pub key: CellKey, + pub old: Option, + pub new: Option, +} + #[cfg(test)] thread_local! { /// How many lookups fell through to set narrowing, for the exact-path test. @@ -215,6 +224,9 @@ pub struct DataStore { /// How many stored cells have each number of coordinates; its largest key /// says when a partial can match only one exact cell. arities: std::collections::BTreeMap, + /// The changes since `record_changes`, or `None` while nobody listens, so an + /// unwatched store keeps no log. + changes: Option>, } /// Resolved through the symbol table and sorted wherever the storage is a @@ -227,6 +239,7 @@ impl std::fmt::Debug for DataStore { index, records, arities: _, + changes: _, } = self; let cells: Vec<_> = cells .iter() @@ -302,7 +315,19 @@ impl DataStore { self.index.entry(*pair).or_default().insert(ikey.clone()); } let width = ikey.0.len(); - let is_new_cell = self.cells.insert(ikey, value).is_none(); + let previous = match &mut self.changes { + Some(log) => { + let previous = self.cells.insert(ikey, value.clone()); + log.push(CellChange { + key, + old: previous.clone(), + new: Some(value), + }); + previous + } + None => self.cells.insert(ikey, value), + }; + let is_new_cell = previous.is_none(); if is_new_cell { *self.arities.entry(width).or_default() += 1; } @@ -316,6 +341,24 @@ impl DataStore { entry.cells += usize::from(is_new_cell); } + /// Start logging every `set` and `remove` as a [`CellChange`], from empty. + pub fn record_changes(&mut self) { + self.changes = Some(Vec::new()); + } + + /// Stop logging and drop what was logged. + pub fn stop_recording_changes(&mut self) { + self.changes = None; + } + + /// The changes since the last drain, oldest first. Logging goes on. + pub fn drain_changes(&mut self) -> Vec { + self.changes + .as_mut() + .map(std::mem::take) + .unwrap_or_default() + } + pub fn get(&self, key: &CellKey) -> Option<&CellValue> { let ikey = self.lookup_key(key)?; self.cells.get(&ikey) @@ -340,7 +383,14 @@ impl DataStore { let Some(ikey) = self.lookup_key(key) else { return; }; - if self.cells.shift_remove(&ikey).is_some() { + if let Some(old) = self.cells.shift_remove(&ikey) { + if let Some(log) = &mut self.changes { + log.push(CellChange { + key: key.clone(), + old: Some(old), + new: None, + }); + } if let Some(n) = self.arities.get_mut(&ikey.0.len()) { *n -= 1; if *n == 0 { @@ -714,6 +764,17 @@ mod cell_key { assert!(!k.matches_partial(&partial)); } + #[test] + fn keys_order_by_their_sorted_coordinates() { + let a = key(&[("A", "z")]); + let b = key(&[("A", "z"), ("B", "a")]); + let c = key(&[("A", "z"), ("B", "b")]); + let d = key(&[("C", "a")]); + let mut keys = vec![d.clone(), c.clone(), a.clone(), b.clone()]; + keys.sort(); + assert_eq!(keys, vec![a, b, c, d]); + } + #[test] fn display_format() { let k = key(&[("Region", "East")]); @@ -723,7 +784,7 @@ mod cell_key { #[cfg(test)] mod data_store { - use super::{CellKey, CellValue, DataStore}; + use super::{CellChange, CellKey, CellValue, DataStore}; fn key(pairs: &[(&str, &str)]) -> CellKey { CellKey::new( @@ -757,6 +818,48 @@ mod data_store { assert_eq!(store.get(&k), Some(&CellValue::Number(99.0))); } + #[test] + fn a_store_reports_no_changes_until_asked_to_record_them() { + let mut store = DataStore::new(); + store.set(key(&[("Region", "East")]), CellValue::Number(1.0)); + assert!(store.drain_changes().is_empty()); + } + + #[test] + fn a_recording_store_reports_a_new_cell_an_overwrite_and_a_removal() { + let mut store = DataStore::new(); + let k = key(&[("Region", "East")]); + store.record_changes(); + store.set(k.clone(), CellValue::Number(1.0)); + store.set(k.clone(), CellValue::Number(2.0)); + store.remove(&k); + store.remove(&k); + let change = |old: Option, new: Option| CellChange { + key: k.clone(), + old: old.map(CellValue::Number), + new: new.map(CellValue::Number), + }; + assert_eq!( + store.drain_changes(), + vec![ + change(None, Some(1.0)), + change(Some(1.0), Some(2.0)), + change(Some(2.0), None) + ], + "removing an absent cell reports nothing" + ); + assert!(store.drain_changes().is_empty(), "a drain empties the log"); + } + + #[test] + fn a_store_that_stopped_recording_reports_nothing_more() { + let mut store = DataStore::new(); + store.record_changes(); + store.stop_recording_changes(); + store.set(key(&[("Region", "East")]), CellValue::Number(1.0)); + assert!(store.drain_changes().is_empty()); + } + #[test] fn remove_evicts_key() { let mut store = DataStore::new(); diff --git a/crates/improvise-incremental/Cargo.toml b/crates/improvise-incremental/Cargo.toml index 3da249b2..2eed040a 100644 --- a/crates/improvise-incremental/Cargo.toml +++ b/crates/improvise-incremental/Cargo.toml @@ -6,9 +6,10 @@ license = "Apache-2.0" description = "The seam between improvise-core and the incremental circuit: the Derived trait, the Eager oracle and the differential harness (ADR 009 clause 2)." # ADR 009 clause 2: depends on improvise-core, never the other way round. The -# `zset` edge arrives with the first backend that uses it (step 3), not before. +# `zset` edge is the cell delta of step 3 (improvise-5r8c). [dependencies] improvise-core = { path = "../improvise-core" } +zset = { path = "../zset" } [dev-dependencies] improvise-io = { path = "../improvise-io" } diff --git a/crates/improvise-incremental/src/delta.rs b/crates/improvise-incremental/src/delta.rs new file mode 100644 index 00000000..8836d1d2 --- /dev/null +++ b/crates/improvise-incremental/src/delta.rs @@ -0,0 +1,261 @@ +//! The cell delta: what a write changed, as a Z-set (ADR 009 clause 3). +//! Reasoning: `context/code/improvise-incremental.md`. + +use std::cmp::Ordering; + +use improvise_core::model::cell::{CellChange, CellKey, CellValue}; +use improvise_core::model::record::RecordHash; +use zset::ZSet; + +/// A cell value that compares by the float rules, with every NaN one value: `-0.0` +/// equals `+0.0`. Built only by [`DeltaValue::new`], which makes both true of the +/// inner value, so the derived-looking order below is a total order. +#[derive(Debug, Clone)] +pub struct DeltaValue(CellValue); + +impl DeltaValue { + pub fn new(value: &CellValue) -> Self { + DeltaValue(match value { + CellValue::Number(n) if n.is_nan() => CellValue::Number(f64::NAN), + CellValue::Number(n) if *n == 0.0 => CellValue::Number(0.0), + other => other.clone(), + }) + } + + pub fn value(&self) -> &CellValue { + &self.0 + } +} + +impl Ord for DeltaValue { + fn cmp(&self, other: &Self) -> Ordering { + match (&self.0, &other.0) { + (CellValue::Number(a), CellValue::Number(b)) => a.total_cmp(b), + (CellValue::Text(a), CellValue::Text(b)) => a.cmp(b), + (CellValue::Error(a), CellValue::Error(b)) => a.cmp(b), + (a, b) => rank(a).cmp(&rank(b)), + } + } +} + +fn rank(value: &CellValue) -> u8 { + match value { + CellValue::Number(_) => 0, + CellValue::Text(_) => 1, + CellValue::Error(_) => 2, + } +} + +impl PartialOrd for DeltaValue { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl PartialEq for DeltaValue { + fn eq(&self, other: &Self) -> bool { + self.cmp(other) == Ordering::Equal + } +} + +impl Eq for DeltaValue {} + +/// What the store's cells changed by: `+(k, v)` new, `-(k, v0) + (k, v1)` overwritten, +/// `-(k, v0)` removed, and nothing for an overwrite that changes no value. +pub type CellDelta = ZSet<(CellKey, DeltaValue)>; + +/// The delta of `changes`, in order. Changes that cancel leave nothing. +pub fn cell_delta<'a>(changes: impl IntoIterator) -> CellDelta { + let mut delta = CellDelta::new(); + for change in changes { + if let Some(old) = &change.old { + delta.insert((change.key.clone(), DeltaValue::new(old)), -1); + } + if let Some(new) = &change.new { + delta.insert((change.key.clone(), DeltaValue::new(new)), 1); + } + } + delta +} + +/// The records `delta` touches, ascending: the record tier of ADR 001's basis. +pub fn moved_records(delta: &CellDelta) -> Vec { + let mut records: Vec = delta + .iter() + .map(|((key, _), _)| RecordHash::of(key)) + .collect(); + records.sort_unstable(); + records.dedup(); + records +} + +#[cfg(test)] +mod tests { + use super::*; + + fn key(region: &str, measure: &str) -> CellKey { + CellKey::new(vec![ + ("Region".into(), region.into()), + ("_Measure".into(), measure.into()), + ]) + } + + fn change(k: &CellKey, old: Option, new: Option) -> CellChange { + CellChange { + key: k.clone(), + old, + new, + } + } + + fn num(n: f64) -> CellValue { + CellValue::Number(n) + } + + fn delta_of(old: Option, new: Option) -> CellDelta { + cell_delta(&[change(&key("East", "Revenue"), old, new)]) + } + + #[test] + fn a_new_cell_an_overwrite_and_a_removal_have_the_clause_3_shapes() { + let k = key("East", "Revenue"); + let at = |v: f64| (k.clone(), DeltaValue::new(&num(v))); + assert_eq!(delta_of(None, Some(num(1.0))), ZSet::singleton(at(1.0), 1)); + assert_eq!( + delta_of(Some(num(1.0)), Some(num(2.0))), + ZSet::from_iter([(at(1.0), -1), (at(2.0), 1)]) + ); + assert_eq!(delta_of(Some(num(1.0)), None), ZSet::singleton(at(1.0), -1)); + } + + #[test] + fn overwriting_a_value_with_one_that_compares_equal_is_the_zero_delta() { + let nans = [ + f64::NAN, + -f64::NAN, + f64::from_bits(0x7ff0_0000_0000_0001), + f64::from_bits(0xfff8_dead_beef_0001), + ]; + for a in nans { + for b in nans { + assert!( + delta_of(Some(num(a)), Some(num(b))).is_zero(), + "NaN {:#x} over NaN {:#x}", + a.to_bits(), + b.to_bits() + ); + } + } + assert!(delta_of(Some(num(0.0)), Some(num(-0.0))).is_zero()); + assert!(delta_of(Some(num(-0.0)), Some(num(0.0))).is_zero()); + let text = || Some(CellValue::Text("x".into())); + assert!(delta_of(text(), text()).is_zero()); + } + + #[test] + fn values_that_differ_by_the_float_rules_stay_different() { + let pairs = [ + (num(1.0), num(1.0000000000000002)), + (num(f64::INFINITY), num(f64::NEG_INFINITY)), + (num(f64::NAN), num(0.0)), + (num(0.0), CellValue::Text("0".into())), + (CellValue::Text("a".into()), CellValue::Error("a".into())), + ]; + for (a, b) in pairs { + assert!( + !delta_of(Some(a.clone()), Some(b.clone())).is_zero(), + "{a:?} over {b:?}" + ); + } + } + + #[test] + fn the_order_is_total_and_agrees_with_equality() { + let values = [ + num(f64::NEG_INFINITY), + num(-1.0), + num(-0.0), + num(0.0), + num(1.0), + num(f64::INFINITY), + num(f64::NAN), + num(-f64::NAN), + CellValue::Text(String::new()), + CellValue::Text("a".into()), + CellValue::Error("a".into()), + ]; + let wrapped: Vec = values.iter().map(DeltaValue::new).collect(); + for a in &wrapped { + for b in &wrapped { + assert_eq!(a.cmp(b), b.cmp(a).reverse()); + assert_eq!(a == b, a.cmp(b) == Ordering::Equal); + for c in &wrapped { + if a <= b && b <= c { + assert!(a <= c, "{a:?} {b:?} {c:?}"); + } + } + } + } + assert_eq!(wrapped[2], wrapped[3], "-0.0 is +0.0"); + assert_eq!(wrapped[6], wrapped[7], "every NaN is one value"); + } + + #[test] + fn numbers_order_by_total_cmp_with_the_canonical_nan_last() { + let ascending = [f64::NEG_INFINITY, -1.5, -0.0, 2.0, f64::INFINITY, -f64::NAN]; + let mut shuffled: Vec = + [2.0, -f64::NAN, -1.5, f64::INFINITY, -0.0, f64::NEG_INFINITY] + .iter() + .map(|n| DeltaValue::new(&num(*n))) + .collect(); + shuffled.sort(); + let want: Vec = ascending + .iter() + .map(|n| DeltaValue::new(&num(*n))) + .collect(); + assert_eq!(shuffled, want); + assert_eq!( + shuffled[2].value().as_f64().map(f64::to_bits), + Some(0), + "and the zero among them reads as +0.0" + ); + } + + #[test] + fn a_wrapped_value_reads_back_with_the_canonical_nan_and_zero() { + let nan = DeltaValue::new(&num(-f64::NAN)); + assert_eq!( + nan.value().as_f64().map(f64::to_bits), + Some(f64::NAN.to_bits()) + ); + let zero = DeltaValue::new(&num(-0.0)); + assert_eq!(zero.value().as_f64().map(f64::to_bits), Some(0)); + } + + #[test] + fn a_change_that_undoes_an_earlier_one_cancels_it() { + let k = key("East", "Revenue"); + let delta = cell_delta(&[ + change(&k, None, Some(num(1.0))), + change(&k, Some(num(1.0)), Some(num(2.0))), + change(&k, Some(num(2.0)), None), + ]); + assert!(delta.is_zero()); + } + + #[test] + fn moved_records_names_each_record_once_whatever_the_measure() { + let delta = cell_delta(&[ + change(&key("East", "Revenue"), None, Some(num(1.0))), + change(&key("East", "Cost"), None, Some(num(2.0))), + change(&key("West", "Cost"), Some(num(3.0)), Some(num(4.0))), + change(&key("South", "Cost"), Some(num(5.0)), Some(num(5.0))), + ]); + let mut want = vec![ + RecordHash::of(&key("East", "Revenue")), + RecordHash::of(&key("West", "Revenue")), + ]; + want.sort_unstable(); + assert_eq!(moved_records(&delta), want, "South wrote nothing new"); + } +} diff --git a/crates/improvise-incremental/src/lib.rs b/crates/improvise-incremental/src/lib.rs index 0f006f9b..69f4a18a 100644 --- a/crates/improvise-incremental/src/lib.rs +++ b/crates/improvise-incremental/src/lib.rs @@ -1,10 +1,12 @@ //! The plug: what the evaluator reads, and the oracle that answers it today. //! Reasoning: `context/code/improvise-incremental.md`. +mod delta; mod eager; mod edit; mod harness; +pub use delta::{CellDelta, DeltaValue, cell_delta, moved_records}; pub use eager::Eager; pub use edit::Edit; pub use harness::{ diff --git a/crates/improvise-incremental/tests/oracle.rs b/crates/improvise-incremental/tests/oracle.rs index 6b3304be..9fa50334 100644 --- a/crates/improvise-incremental/tests/oracle.rs +++ b/crates/improvise-incremental/tests/oracle.rs @@ -7,7 +7,8 @@ use improvise_core::model::cell::{CellKey, CellValue}; use improvise_core::model::{Aggregation, Model}; use improvise_core::view::Axis; use improvise_incremental::{ - Derived, Divergence, Eager, Edit, Maintained, Probe, differential, probes, same_value, + CellDelta, DeltaValue, Derived, Divergence, Eager, Edit, Maintained, Probe, cell_delta, + differential, probes, same_value, }; use proptest::prelude::*; use std::path::Path; @@ -594,6 +595,50 @@ fn any_value() -> impl Strategy { ] } +/// The store as a Z-set, every cell at weight one. +fn snapshot(model: &Model) -> CellDelta { + model + .data + .iter_cells() + .map(|(k, v)| ((k, DeltaValue::new(v)), 1)) + .collect() +} + +/// Values the delta must treat as the float rules do: both zeros, several NaNs. +fn delta_value() -> impl Strategy { + prop_oneof![ + (-2i32..3).prop_map(|n| CellValue::Number(f64::from(n))), + Just(CellValue::Number(-0.0)), + Just(CellValue::Number(f64::NAN)), + Just(CellValue::Number(-f64::NAN)), + Just(CellValue::Number(f64::from_bits(0x7ff0_0000_0000_0001))), + prop::sample::select(vec!["n", "x"]).prop_map(|t| CellValue::Text(t.into())), + ] +} + +fn cell_edit() -> impl Strategy { + prop_oneof![ + (any_key(), delta_value()).prop_map(|(k, v)| Edit::SetCell(k, v)), + any_key().prop_map(Edit::ClearCell), + ] +} + +proptest! { + #[test] + fn the_cell_delta_of_an_edit_is_the_store_after_minus_the_store_before( + edits in prop::collection::vec(cell_edit(), 0..12) + ) { + let mut model = hand_model(); + model.data.record_changes(); + for edit in &edits { + let before = snapshot(&model); + edit.apply(&mut model); + let delta = cell_delta(&model.data.drain_changes()); + prop_assert_eq!(delta, &snapshot(&model) - &before, "after {:?}", edit); + } + } +} + fn any_edit() -> impl Strategy { prop_oneof![ (any_key(), any_value()).prop_map(|(k, v)| Edit::SetCell(k, v)),