diff --git a/context/code/source.md b/context/code/source.md index 5eb7d12f..164b3a34 100644 --- a/context/code/source.md +++ b/context/code/source.md @@ -129,5 +129,6 @@ backend a Revenue of 128 and the other 38, and the second then disagreed with it `Summary::from_parts(Numbers::from_parts(count, exact_sum, least, greatest)?, Agreement)?`. `Agreement` is what the non-number values hold: `None`, `One(v)` or `Mixed`. Both constructors return a `Result` and **refuse** a state no sequence of values could produce (extremes with no numbers, a NaN extreme, -least above greatest, one extreme without the other, a number among the "others"), because repairing +least above greatest, one extreme without the other, a sum with no numbers, no extremes beside a sum +that is not NaN, a number among the "others"), because repairing it would hide an engine's bug. diff --git a/context/test-evidence/core.md b/context/test-evidence/core.md index b4969633..d9842ddd 100644 --- a/context/test-evidence/core.md +++ b/context/test-evidence/core.md @@ -2317,7 +2317,8 @@ lost). The first two also fail `aggregating_differing_text_values_is_an_explicit Review of #356 (Opus): the pool gained `1e16`, `1.0`, `-1e16`, `0.1` and `0.2`. A merge rounding each part's sum before pooling, keeping the zero sign, changed 514 production answers while the workspace passed; that rival now fails this test, `numbers_pool_what_the_formula_aggregate_pools` (a NaN-free split now, -compared as bits), `summaries_built_from_parts_merge_as_the_union_does` and the fixture test below, by name. +compared as bits), `summaries_built_from_parts_merge_as_the_union_does` and the fixture test below, by +name: four tests, for each of the three round-then-pool variants. A read of several parts is a merge. The test merges every ordered pair of sequences of up to two values and compares with the reference of the concatenation. Rivals: the least not merged, the @@ -2426,12 +2427,14 @@ Review of #356 (Opus, Codex): `from_parts` repaired some impossible states and a The earlier entry's claim that a NaN-extreme rival failed `parts_that_contradict_themselves_...` was false: that test passed with the filter removed. Rivals now, each failing this test alone: NaN extreme allowed, least above greatest allowed, extremes with no numbers allowed, a number accepted among the -others. +others, a sum with no numbers allowed, and no extremes beside a sum that is not NaN allowed. ### the_exact_sum_of_a_pool_of_cancelling_values_is_not_the_sum_of_rounded_parts An exactness test needs inputs whose float sum is inexact and no NaN. `1e16 + 1.0 - 1e16` rounded part -by part is `0.0`; pooled exactly it is `1.0`. This pins the fixture rather than the code. +by part is `0.0`; pooled exactly it is `1.0`. The merge runs in both orders, so whichever part a rival +rounds is the inexact one: rounding only the other part, only self, or both each fail it (the first +version merged one way and a rival rounding only the exact part passed it; Opus caught that). ### keys_of_a_store_are_its_cells_keys_without_the_values diff --git a/crates/improvise-core/src/model/source.rs b/crates/improvise-core/src/model/source.rs index b36a8e55..740489b4 100644 --- a/crates/improvise-core/src/model/source.rs +++ b/crates/improvise-core/src/model/source.rs @@ -70,7 +70,13 @@ impl Numbers { greatest: Option, ) -> Result { let bad = |why| Err(BadParts(why)); + if count == 0 && sum != ExactSum::new() { + return bad("a sum with no numbers"); + } match (least, greatest) { + (None, None) if count > 0 && !sum.value().is_nan() => { + return bad("no extremes but a sum that is not NaN"); + } (None, None) => {} (Some(_), Some(_)) if count == 0 => return bad("extremes with no numbers"), (Some(l), Some(g)) if l.is_nan() || g.is_nan() => return bad("a NaN extreme"), @@ -535,13 +541,15 @@ mod tests { #[test] fn the_exact_sum_of_a_pool_of_cancelling_values_is_not_the_sum_of_rounded_parts() { let rounded = [1e16, 1.0].iter().sum::() + [-1e16].iter().sum::(); - let mut pooled = Numbers::of([1e16, 1.0]); - pooled.merge(&Numbers::of([-1e16])); assert_eq!( rounded, 0.0, - "the fixture is inexact: summing rounded parts loses the 1" + "the fixture is inexact: rounded parts lose the 1" ); - assert_eq!(pooled.total(), 1.0); + for (first, second) in [([1e16, 1.0], [-1e16, 0.0]), ([-1e16, 0.0], [1e16, 1.0])] { + let mut pooled = Numbers::of(first); + pooled.merge(&Numbers::of(second)); + assert_eq!(pooled.total(), 1.0, "{first:?} then {second:?}"); + } } fn pair(c: &str, i: &str) -> (String, String) { @@ -854,7 +862,19 @@ mod tests { refused(Numbers::from_parts(2, sum(), Some(1.0), None)), "one extreme without the other" ); - let ok = Numbers::from_parts(2, sum(), None, None).unwrap(); + let mut one = ExactSum::new(); + one.add(1.0, 1); + assert_eq!( + refused(Numbers::from_parts(0, one.clone(), None, None)), + "a sum with no numbers" + ); + assert_eq!( + refused(Numbers::from_parts(2, one, None, None)), + "no extremes but a sum that is not NaN" + ); + let mut nan = ExactSum::new(); + nan.add(f64::NAN, 1); + let ok = Numbers::from_parts(2, nan, None, None).unwrap(); assert!( ok.reduce(&AggFunc::Min).unwrap().is_nan(), "all NaN: no extremes, a count"