diff --git a/benches/grid_projection.rs b/benches/grid_projection.rs index 946610e4..83e73069 100644 --- a/benches/grid_projection.rs +++ b/benches/grid_projection.rs @@ -62,7 +62,8 @@ fn grouped_app(days: usize, per_group: usize, products: usize, measures: usize) wb.add_category("Product").unwrap(); for d in 0..days { let group = format!("G{}", d / per_group); - wb.add_item_in_group("Day", format!("Day{d}"), group).unwrap(); + wb.add_item_in_group("Day", format!("Day{d}"), group) + .unwrap(); } for p in 0..products { wb.add_item("Product", format!("Product{p}")).unwrap(); diff --git a/context/code/command.md b/context/code/command.md index 6ec985ca..8b05d17a 100644 --- a/context/code/command.md +++ b/context/code/command.md @@ -1805,11 +1805,44 @@ one `CellKey` per member, the cross product of every spanned category's covered items (3,000 keys for an 18k column), and looked each one up. Now `group_expansion` returns the expansion as data: a `base` key and the covered items of each expanded category. `fold_stored` hands that to -`DataStore::spanned_values`, which narrows by `base` through the index once and -filters each candidate by membership. In `Exact` mode a candidate must also -carry no coordinate beyond those, which is what `get(member)` answered. Only a -formula still enumerates members (`contexts_of`), since its references are -evaluated per context. +`DataStore::spanned_values`, which narrows through the index and filters each +candidate by membership. In `Exact` mode a candidate must also carry no +coordinate beyond those, which is what `get(member)` answered. Only a formula +still enumerates members (`contexts_of`), since its references are evaluated +per context. + +**How it narrows is a choice, and the first version got it wrong +(improvise-jq0m).** Narrowing by `base` alone is right for a column total, which +spans *every* item of each row category. It is wrong for a collapsed group, +which spans two or seven items of a category with hundreds: `base` drops the +grouped category entirely, so every fold cell scanned every record of the +column, and a grid of small groups went quadratic. The review measured 1000 +days in pairs at 11–15.7 s a keypress, against 0.44–0.51 s before #239. So +`spanned_positions` now prices both narrowings off the index's own set sizes +and takes the cheaper: once by `base`, or once per covered item of the span +with the fewest items, each intersected with `base`. The per-item positions are +disjoint (a record has one item per category) and are sorted afterwards, so +both paths return the store's order. A `base` naming a name the store never +interned matches nothing on both paths. + +The bench now carries the grouped cases, which it could not see before: Day in +collapsed groups on Row, with Product, and `_Measure` on Column. The machine is +an Apple M4 Max (16 cores, 64 GB) at load average 15–63. The figures are +keypress (`whole-grid + col_widths`), min-of-N, from two alternating rounds, +the second reversed. Each cell gives both rounds: + +| case | before #239 (`5932ad74`) | `f668ddc9` | jq0m | jq0m ÷ before | +|---|---|---|---|---| +| 4.8k dense | 33.9 / 39.1 ms | 40.7 / 40.2 ms | 36.3 / 33.3 ms | 1.07 / 0.85 | +| 18k dense | 237 / 239 ms | 238 / 232 ms | 196 / 211 ms | 0.83 / 0.88 | +| 90k dense | 3.04 / 3.46 s | 3.24 / 2.70 s | 2.68 / 2.84 s | 0.88 / 0.82 | +| 364 days in weeks | 483 / 415 ms | 1.42 / 1.18 s | 356 / 399 ms | 0.74 / 0.96 | +| 364 days in pairs | 715 / 539 ms | 5.02 / 4.62 s | 471 / 518 ms | 0.66 / 0.96 | +| 1000 days in pairs | 406 / 399 ms | 10.8 / 20.2 s | 352 / 364 ms | 0.87 / 0.91 | + +The grouped cases now come in **below** the pre-#239 figures. The old +per-member path looked up each member through a full `CellKey`, and the +per-item narrowing intersects index sets directly. Two levers were **not** taken. Combining the walk's row values for decomposable aggregators was rejected under §5 as a second derivation of the total. It is diff --git a/context/test-evidence/core.md b/context/test-evidence/core.md index 2a30c19a..1df4babf 100644 --- a/context/test-evidence/core.md +++ b/context/test-evidence/core.md @@ -1537,3 +1537,30 @@ Three faults were injected, each run against the full `improvise-core` and (collapsed-group, hidden-row and CSV-agreement tests). - A category the store never interned is skipped instead of matching nothing. **Only this test fails.** + +### a_span_finds_what_its_members_find_in_store_order + +improvise-jq0m. `spanned_values` now chooses between two narrowings, and a +fixed example can only ever take one of them. This property builds random +stores over `R`, `P` and `M`. Some records lack `R`, some carry an extra `X`, +and `r6` is a covered item the store never interned. It spans `R` and sometimes +`P`, in both modes, and compares the result with the per-member definition, +**in store order**: the values the members' own lookups find, read back in +`iter_cells` order. Store and span sizes vary across cases, so both narrowings +are taken. + +Three faults were injected, each run against the full `improvise-core` and +`improvise-command` suites: + +- The per-item narrowing drops the base, so it keeps each item's pair alone. + This test and `a_spanned_scan_finds_what_a_lookup_per_member_finds` fail. + **Nothing in either crate's grid or command tests does.** Those fixtures never + hold a second value of the base's coordinate within a small span. +- The base narrowing drops the base. This test fails, along with eight + fold and totals tests. +- The per-item positions are not sorted after they are merged. **Only this + test fails.** Values stay correct and only their order moves, which changes + a Sum or Avg in the last bits. + +The regression itself is a cost, which no test here can see. The evidence for +it is the `grid_projection` bench's grouped cases, added for this ticket. diff --git a/crates/improvise-core/src/model/cell.rs b/crates/improvise-core/src/model/cell.rs index 19254613..dbae8faa 100644 --- a/crates/improvise-core/src/model/cell.rs +++ b/crates/improvise-core/src/model/cell.rs @@ -484,7 +484,11 @@ impl DataStore { .unwrap_or_default() } - fn narrowed(&self, base: &[(Symbol, Symbol)], wanted: &[(Symbol, HashSet)]) -> Vec { + fn narrowed( + &self, + base: &[(Symbol, Symbol)], + wanted: &[(Symbol, HashSet)], + ) -> Vec { let with = |cat: Symbol, item: Symbol| [base, &[(cat, item)]].concat(); let per_item: Vec> = wanted .iter()