test(incremental): formula cache reads, wide sets, NaN and the evidence for step 5 (improvise-shbc)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
committed by
Edward Langley
parent
7e3c6b0c30
commit
933c9254a0
@@ -37,9 +37,8 @@ fn walk(expr: &Expr, names: &mut BTreeSet<String>, filters: &mut BTreeSet<String
|
||||
}
|
||||
}
|
||||
|
||||
/// What one formula reads: the names in its expression and in the formulas those names
|
||||
/// target, and the categories that puts in play. A cell the formula can read agrees with
|
||||
/// the stem everywhere but on `cats`.
|
||||
/// What one formula reads: names, through the formulas they target, and the categories
|
||||
/// that puts in play. Reasoning: `context/code/improvise-incremental.md`, step 5.
|
||||
#[derive(Debug, Clone, Default, PartialEq, Eq)]
|
||||
pub struct Reads {
|
||||
pub names: BTreeSet<String>,
|
||||
@@ -59,11 +58,15 @@ fn reads_of(model: &Model, at: usize) -> Reads {
|
||||
walk(&f.expr, &mut names, &mut filters);
|
||||
filters.extend(f.filter.iter().map(|x| x.category.clone()));
|
||||
reads.cats.insert(f.target_category.clone());
|
||||
reads.names.insert(f.target.clone());
|
||||
reads.cats.extend(filters);
|
||||
for name in names {
|
||||
let resolved = model.find_item_category(&name).map(String::from);
|
||||
reads.cats.extend(resolved);
|
||||
let targets = formulas.iter().enumerate().filter(|(_, g)| g.target == name);
|
||||
let targets = formulas
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter(|(_, g)| g.target == name);
|
||||
todo.extend(targets.map(|(j, _)| j));
|
||||
reads.names.insert(name);
|
||||
}
|
||||
@@ -213,15 +216,19 @@ impl Stems {
|
||||
pub fn build(model: &Model, none_cats: &[String]) -> Stems {
|
||||
let formulas = model.formulas();
|
||||
let reads: Vec<Reads> = (0..formulas.len()).map(|i| reads_of(model, i)).collect();
|
||||
let targets: BTreeSet<&str> = formulas.iter().map(|f| f.target_category.as_str()).collect();
|
||||
let wide = targets.len() > 1 || formulas.iter().any(|f| {
|
||||
let (mut names, mut filters) = (BTreeSet::new(), BTreeSet::new());
|
||||
walk(&f.expr, &mut names, &mut filters);
|
||||
names.iter().any(|n| {
|
||||
let cat = model.find_item_category(n);
|
||||
cat.is_some_and(|c| c != f.target_category && !none_cats.iter().any(|x| x == c))
|
||||
})
|
||||
});
|
||||
let targets: BTreeSet<&str> = formulas
|
||||
.iter()
|
||||
.map(|f| f.target_category.as_str())
|
||||
.collect();
|
||||
let wide = targets.len() > 1
|
||||
|| formulas.iter().any(|f| {
|
||||
let (mut names, mut filters) = (BTreeSet::new(), BTreeSet::new());
|
||||
walk(&f.expr, &mut names, &mut filters);
|
||||
names.iter().any(|n| {
|
||||
let cat = model.find_item_category(n);
|
||||
cat.is_some_and(|c| c != f.target_category && !none_cats.iter().any(|x| x == c))
|
||||
})
|
||||
});
|
||||
let mut stems = Stems {
|
||||
none_cats: none_cats.to_vec(),
|
||||
target_cats: formulas.iter().map(|f| f.target_category.clone()).collect(),
|
||||
@@ -313,7 +320,12 @@ impl Stems {
|
||||
pub fn step(&mut self, changes: &[&CellChange]) -> Moved {
|
||||
let presence: Vec<(&CellKey, i64)> = changes
|
||||
.iter()
|
||||
.map(|c| (&c.key, i64::from(c.new.is_some()) - i64::from(c.old.is_some())))
|
||||
.map(|c| {
|
||||
(
|
||||
&c.key,
|
||||
i64::from(c.new.is_some()) - i64::from(c.old.is_some()),
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
let (born, gone) = self.count(&presence);
|
||||
let mut dirty: BTreeSet<CellKey> = born.into_iter().collect();
|
||||
|
||||
@@ -266,7 +266,8 @@ impl Incremental {
|
||||
let new = self.evaluate_cell(key);
|
||||
if self.model.formula_cell(key) != new.as_ref() {
|
||||
moving.push(key);
|
||||
self.model.set_formula_cell(key.clone(), new.or_else(|| self.keep(key)));
|
||||
self.model
|
||||
.set_formula_cell(key.clone(), new.or_else(|| self.keep(key)));
|
||||
}
|
||||
}
|
||||
if moving.is_empty() {
|
||||
|
||||
@@ -1,7 +0,0 @@
|
||||
# Seeds for failure cases proptest has generated in the past. It is
|
||||
# automatically read and these particular cases re-run before any
|
||||
# novel cases are generated.
|
||||
#
|
||||
# It is recommended to check this file in to source control so that
|
||||
# everyone who runs the test benefits from these saved cases.
|
||||
cc afa9f2dae4083512a61fc43079707ae6fef3a5893d4e31291f5a8d7a1abb0021 # shrinks to order = [10], by_view_none = false
|
||||
@@ -52,8 +52,14 @@ fn a_formula_that_keeps_moving_for_sixteen_passes_is_circular() {
|
||||
])
|
||||
};
|
||||
let circular = Some(CellValue::Error("circular".into()));
|
||||
assert_eq!(backend.value(&at("Loop"), Aggregation::OverHidden), circular);
|
||||
assert_eq!(backend.value(&at("Ping"), Aggregation::OverHidden), circular);
|
||||
assert_eq!(
|
||||
backend.value(&at("Loop"), Aggregation::OverHidden),
|
||||
circular
|
||||
);
|
||||
assert_eq!(
|
||||
backend.value(&at("Ping"), Aggregation::OverHidden),
|
||||
circular
|
||||
);
|
||||
let unseeded = Some(CellValue::Error("ref:Loop".into()));
|
||||
let b = CellKey::new(vec![
|
||||
("Product".into(), "B".into()),
|
||||
@@ -114,7 +120,11 @@ fn a_cell_edit_evaluates_the_one_stem_that_reads_it_whatever_the_table_size() {
|
||||
backend.apply(&bump("East", "P3", "Revenue", 50.0));
|
||||
let spent = backend.meter();
|
||||
assert_eq!(spent.stems - before.stems, 1, "{products} products");
|
||||
assert_eq!(spent.formula_cells - before.formula_cells, 2, "Profit and Margin");
|
||||
assert_eq!(
|
||||
spent.formula_cells - before.formula_cells,
|
||||
2,
|
||||
"Profit and Margin"
|
||||
);
|
||||
assert_eq!(spent.rebuilds, before.rebuilds, "no stem index is rebuilt");
|
||||
}
|
||||
}
|
||||
@@ -131,16 +141,112 @@ fn a_formula_edit_evaluates_every_stem_once_and_rebuilds_the_index_once() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_formula_replaced_in_place_is_evaluated_afresh_and_a_signed_zero_is_a_change() {
|
||||
fn a_formula_replaced_in_place_is_evaluated_afresh() {
|
||||
let wb = formula_workbook();
|
||||
let none = vec!["Region".to_string()];
|
||||
let swapped = parse_formula("Profit = Revenue + Cost", "_Measure").unwrap();
|
||||
for by in [Evaluation::Ordered, Evaluation::Plain] {
|
||||
same_model(&wb.model, &none, &[Edit::AddFormula(swapped.clone())], by).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn overwriting_zero_with_negative_zero_changes_what_a_formula_reads() {
|
||||
let wb = formula_workbook();
|
||||
let none = vec!["Region".to_string()];
|
||||
let mut edits = vec![formula_edit(16)];
|
||||
edits.extend(["East", "West"].map(|r| bump(r, "A", "Units", 0.0)));
|
||||
edits.extend(["East", "West"].map(|r| bump(r, "A", "Units", -0.0)));
|
||||
for by in [Evaluation::Ordered, Evaluation::Plain] {
|
||||
same_model(&wb.model, &none, &edits, by).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_filter_on_a_category_the_stem_has_reads_the_cells_of_other_stems() {
|
||||
let wb = formula_workbook();
|
||||
let none = vec!["Region".to_string()];
|
||||
let edits = [
|
||||
Edit::AddFormula(swapped),
|
||||
formula_edit(16),
|
||||
bump("East", "A", "Units", 0.0),
|
||||
bump("East", "A", "Units", -0.0),
|
||||
bump("West", "A", "Units", -0.0),
|
||||
bump("East", "B", "Revenue", 5.0),
|
||||
bump("West", "B", "Cost", 2.0),
|
||||
formula_edit(9),
|
||||
bump("East", "A", "Revenue", 100.0),
|
||||
];
|
||||
for by in [Evaluation::Ordered, Evaluation::Plain] {
|
||||
same_model(&wb.model, &none, &edits, by).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn two_target_categories_read_each_others_stems_whatever_the_order_they_are_added_in() {
|
||||
let wb = formula_workbook();
|
||||
let none = none_of(&wb);
|
||||
let edits = [
|
||||
formula_edit(14),
|
||||
bump("East", "A", "Loop", 0.0),
|
||||
formula_edit(6),
|
||||
];
|
||||
for by in [Evaluation::Ordered, Evaluation::Plain] {
|
||||
same_model(&wb.model, &none, &edits, by).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_name_in_another_category_reads_another_stems_cell() {
|
||||
let mut wb = formula_workbook();
|
||||
wb.model.category_mut("Product").unwrap().add_item("B");
|
||||
let none = vec!["Region".to_string()];
|
||||
let pf = parse_formula("Pf = IF(Revenue > 1000, B, Revenue)", "_Measure").unwrap();
|
||||
let edits = [
|
||||
bump("East", "A", "Revenue", 5000.0),
|
||||
bump("East", "B", "Revenue", 7.0),
|
||||
Edit::AddFormula(pf),
|
||||
bump("East", "B", "Revenue", 9.0),
|
||||
];
|
||||
for by in [Evaluation::Ordered, Evaluation::Plain] {
|
||||
same_model(&wb.model, &none, &edits, by).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_formula_cell_holding_nan_is_circular_and_the_cells_that_read_it_keep_the_nan_they_read() {
|
||||
let wb = formula_workbook();
|
||||
let none = vec!["Region".to_string()];
|
||||
let nan = bump("East", "A", "Revenue", f64::NAN);
|
||||
let mut backend = Incremental::new(wb.model.clone(), none.clone());
|
||||
backend.apply(&nan);
|
||||
let at = |m: &str| {
|
||||
CellKey::new(vec![
|
||||
("Product".into(), "A".into()),
|
||||
("_Measure".into(), m.into()),
|
||||
])
|
||||
};
|
||||
let circular = Some(CellValue::Error("circular".into()));
|
||||
assert_eq!(
|
||||
backend.value(&at("Profit"), Aggregation::OverHidden),
|
||||
circular
|
||||
);
|
||||
assert_eq!(
|
||||
backend.value(&at("Margin"), Aggregation::OverHidden),
|
||||
circular
|
||||
);
|
||||
for by in [Evaluation::Ordered, Evaluation::Plain] {
|
||||
same_model(&wb.model, &none, std::slice::from_ref(&nan), by).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_new_aggregation_for_the_measure_a_filtered_formula_falls_back_to_changes_it() {
|
||||
let wb = formula_workbook();
|
||||
let none = vec!["Region".to_string()];
|
||||
let edits = [
|
||||
formula_edit(4),
|
||||
bump("East", "A", "Bonus", 3.0),
|
||||
bump("West", "A", "Bonus", 4.0),
|
||||
Edit::SetMeasureAgg {
|
||||
measure: "Bonus".into(),
|
||||
func: improvise_core::formula::ast::AggFunc::Count,
|
||||
},
|
||||
];
|
||||
for by in [Evaluation::Ordered, Evaluation::Plain] {
|
||||
same_model(&wb.model, &none, &edits, by).unwrap();
|
||||
|
||||
@@ -237,7 +237,7 @@ pub fn hidden_counted_workbook() -> improvise_core::workbook::Workbook {
|
||||
|
||||
/// Formulas the random tests add and remove, as `(text, target category)`: chains, a cycle,
|
||||
/// aggregates over expressions, filters, a ratio, and two that target Region.
|
||||
pub const FORMULA_POOL: [(&str, &str); 17] = [
|
||||
pub const FORMULA_POOL: [(&str, &str); 18] = [
|
||||
("Profit = Revenue - Cost", "_Measure"),
|
||||
("Margin = Profit / Revenue", "_Measure"),
|
||||
("Weighted = SUM(Revenue * Units)", "_Measure"),
|
||||
@@ -255,6 +255,7 @@ pub const FORMULA_POOL: [(&str, &str); 17] = [
|
||||
("Total = East + West", "Region"),
|
||||
("EastRev = Revenue", "Region"),
|
||||
("Recip = Units ^ -1", "_Measure"),
|
||||
("Pa = A + Revenue", "_Measure"),
|
||||
];
|
||||
|
||||
pub fn formula_edit(index: usize) -> Edit {
|
||||
@@ -280,19 +281,16 @@ pub fn formula_workbook() -> improvise_core::workbook::Workbook {
|
||||
for r in ["East", "West"] {
|
||||
wb.add_item("Region", r).unwrap();
|
||||
}
|
||||
for p in ["A"] {
|
||||
wb.add_item("Product", p).unwrap();
|
||||
}
|
||||
wb.add_item("Product", "A").unwrap();
|
||||
for m in ["Revenue", "Cost", "Units"] {
|
||||
wb.add_item("_Measure", m).unwrap();
|
||||
}
|
||||
let mut n = 1.0;
|
||||
for r in ["East", "West"] {
|
||||
for p in ["A"] {
|
||||
for (m, scale) in [("Revenue", 10.0), ("Cost", 4.0), ("Units", 2.0)] {
|
||||
wb.model.set_cell(key(r, p, m), CellValue::Number(scale * n));
|
||||
n += 1.0;
|
||||
}
|
||||
for (m, scale) in [("Revenue", 10.0), ("Cost", 4.0), ("Units", 2.0)] {
|
||||
wb.model
|
||||
.set_cell(key(r, "A", m), CellValue::Number(scale * n));
|
||||
n += 1.0;
|
||||
}
|
||||
}
|
||||
let profit = parse_formula("Profit = Revenue - Cost", "_Measure").unwrap();
|
||||
@@ -311,17 +309,27 @@ pub fn bump(region: &str, product: &str, measure: &str, to: f64) -> Edit {
|
||||
}
|
||||
|
||||
/// Both backends told every edit; their models, formula cache included, must print alike.
|
||||
pub fn same_model(model: &Model, none: &[String], edits: &[Edit], by: Evaluation) -> Result<(), String> {
|
||||
pub fn same_model(
|
||||
model: &Model,
|
||||
none: &[String],
|
||||
edits: &[Edit],
|
||||
by: Evaluation,
|
||||
) -> Result<(), String> {
|
||||
let mut eager = Eager::new(model.clone(), none.to_vec());
|
||||
let mut incremental = Incremental::new(model.clone(), none.to_vec()).with_evaluation(by);
|
||||
for edit in edits {
|
||||
eager.apply(edit);
|
||||
incremental.apply(edit);
|
||||
}
|
||||
let (want, got) = (format!("{:?}", eager.model()), format!("{:?}", incremental.model()));
|
||||
let (want, got) = (
|
||||
format!("{:?}", eager.model()),
|
||||
format!("{:?}", incremental.model()),
|
||||
);
|
||||
match want == got {
|
||||
true => Ok(()),
|
||||
false => Err(format!("after {edits:?}:\n eager {want}\n incremental {got}")),
|
||||
false => Err(format!(
|
||||
"after {edits:?}:\n eager {want}\n incremental {got}"
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -339,7 +347,7 @@ fn any_value() -> impl Strategy<Value = CellValue> {
|
||||
pub fn any_edit() -> impl Strategy<Value = Edit> {
|
||||
let regions = prop::sample::select(vec!["East", "West", "North"]);
|
||||
let products = prop::sample::select(vec!["A", "B", "C"]);
|
||||
let measures = prop::sample::select(vec!["Revenue", "Cost", "Units", "Loop", "Pong"]);
|
||||
let measures = prop::sample::select(vec!["Revenue", "Cost", "Units", "Loop", "Pong", "Pa"]);
|
||||
let at = (regions, products, measures);
|
||||
let funcs = vec![
|
||||
AggFunc::Sum,
|
||||
@@ -370,10 +378,12 @@ pub fn same_orders(model: &Model, none: &[String], edits: &[Edit]) -> Result<(),
|
||||
ordered.apply(edit);
|
||||
plain.apply(edit);
|
||||
}
|
||||
let (want, got) = (format!("{:?}", plain.model()), format!("{:?}", ordered.model()));
|
||||
let (want, got) = (
|
||||
format!("{:?}", plain.model()),
|
||||
format!("{:?}", ordered.model()),
|
||||
);
|
||||
match want == got {
|
||||
true => Ok(()),
|
||||
false => Err(format!("after {edits:?}:\n plain {want}\n ordered {got}")),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user