fit: route per record and aggregate, never route on the aggregate
improvise-4zl7. A tree is fitted on individual records; an improvise formula is evaluated at a CellKey, and an aggregating cell used to aggregate each Expr::Ref first and evaluate the IF once on the totals. That is the same answer for a linear formula and a different one for anything that routes, so every fitted tree meant one thing on a record row and something else one row up. The user ruled: aggregate the rule over the members. The rule needed no new aggregate vocabulary. SUM(e) already means "fold e over the members" and simply did not: eval_agg specialised Expr::Ref and, for every other inner expression, summed the raw matching cells and ignored the expression entirely. It now folds — one entry per member record, the expression evaluated at that record's own coordinates. eval_agg is shared by eval_expr and eval_expr_cached, so the fold has exactly one home; each evaluator passes its own recursion in. Expr::Is(Filter) is the new arm in both evaluators: `Cat IS Item` is 1.0 when the cell's own Cat coordinate is Item, keeping the evaluator f64-only and reusing Formula::filter's own context.get/unwrap_or(false). A grammar production in primary, before ref_expr, with both operands through improvise_formula::quote's one home; IS joins the keyword guard. The emitter reads feature index -> role off preprocess's plan rather than reconstructing it: gp_tree::data::feature_roles is the one statement of the expansion order and encode_row now drives off it too. improvise_gp::Features carries one Spelling per feature — Measure, Coordinate or Artefact — decided by role and column, never by the name's shape. A one-hot over a real category is emitted as `Region IS North` and accepted; col.missing and the date parts stay Artefact and stay refused (improvise-wd6n). fit-measure on a document a category explains now lands instead of refusing, and the refusal no longer depends on which features the search happened to pick. Evidence: eight hand-computed tests in improvise-core over a four-record fixture whose Cost straddles the threshold inside each region, so route-then-aggregate and aggregate-then-route differ at every level. Two faults injected and watched to fail — reverting the fold fails 7 of 8, and a constant-false eval_is fails exactly the 3 coordinate tests. Reordering feature_roles fails both it and encode_row_replays_the_training_expansion. The fold is O(members) per evaluation per fixed-point pass and unbenchmarked: improvise-9iyv. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Edward Langley
@@ -433,3 +433,71 @@ Registration is guaranteed for every model category: `Workbook::new`,
|
||||
`axis_of`. Use `try_axis_of` when the name comes from anywhere else —
|
||||
`cycle_axis` does, and no-ops on an unknown category to match `set_axis`
|
||||
(improvise-t8s).
|
||||
|
||||
## 13. Route then aggregate: `SUM(<expr>)` folds over the member records
|
||||
|
||||
improvise-4zl7, and the one place the evaluator reaches past the cell it was
|
||||
asked about.
|
||||
|
||||
**The problem.** A formula is evaluated at a `CellKey`. In an aggregating cell
|
||||
`evaluate_aggregated` aggregates each `Expr::Ref` *first* and evaluates the
|
||||
expression once on the totals. That is exactly right for a linear formula —
|
||||
`SUM(Revenue) - SUM(Cost)` and `SUM(Revenue - Cost)` are the same number — and
|
||||
exactly wrong for anything that **routes**. `IF(Cost < 50, 1, 10)` over records
|
||||
costing 10 and 100 answers `1 + 10 = 11` per record and `10` on the total,
|
||||
because *aggregate-then-route is not route-then-aggregate*. Every fitted tree
|
||||
(improvise-8xhy, improvise-vx3) is such a formula, so it meant one thing on a
|
||||
record row and something else one row up. The user's ruling was to **aggregate
|
||||
the rule over the members**.
|
||||
|
||||
**The mechanism, and why it needed no new vocabulary.** `SUM(e)` already means
|
||||
"fold `e` over the members". It simply did not: `eval_agg` specialised
|
||||
`Expr::Ref` and, for every other inner expression, summed the raw cells matching
|
||||
the partial key and *ignored the expression entirely*. It now folds — one entry
|
||||
per member record, the expression evaluated at that record's own coordinates:
|
||||
|
||||
```
|
||||
partial = context.without(target_category) (plus any inline WHERE)
|
||||
records = distinct(matching_cells(partial).map(key.without("_Measure")))
|
||||
values = records.map(|r| eval(inner, context.with(every coord of r)))
|
||||
```
|
||||
|
||||
Three consequences worth stating.
|
||||
|
||||
The rule is **written once**. `eval_agg` is shared by `eval_expr` and
|
||||
`eval_expr_cached`, so the fold has one home; the two evaluators pass their own
|
||||
recursion in as `eval_at`. Only `Expr::Is` needed an arm in each, and the two
|
||||
arms are one line apiece.
|
||||
|
||||
The `Expr::Ref` fast path **stays**, and it is the same fold specialised: a
|
||||
record's `Ref` is that record's own cell, so summing them is summing the
|
||||
matching values. It differs in exactly one place — a record with no such cell is
|
||||
*skipped* there and an **error** in the fold. That is deliberate: a bare
|
||||
`SUM(Revenue)` asks for the values that exist, while `SUM(<expr>)` asks the
|
||||
expression of each member, and an expression that cannot be evaluated is an
|
||||
error rather than a zero. Silently dropping members is how a routed sum would
|
||||
quietly answer for a subset.
|
||||
|
||||
Records come out in `DataStore` insertion order, deduplicated by an `IndexSet`,
|
||||
for §8's reason — hash order here would make a fitted cell drift in its last
|
||||
bits between processes.
|
||||
|
||||
**Interaction with the fixed point (§3) and `formula_cache`.** None of this is
|
||||
new state. The fold reads *raw* data through the same `evaluate_depth` /
|
||||
`aggregate_raw` the non-folding path uses, so a formula cell still depends only
|
||||
on raw cells and on other formula cells' cached values; `recompute_formulas`
|
||||
iterates the same stems to the same fixed point, and `formula_cache` still holds
|
||||
exactly the aggregated formula cells it held before. The fold does not cache and
|
||||
does not memoise: it is O(members) per evaluation per pass, so a routed formula
|
||||
over a large document is the one place this layer got materially more
|
||||
expensive, and it is unbenchmarked.
|
||||
|
||||
**`Expr::Is(Filter)` — a coordinate predicate as a number.** `Cat IS Item` is
|
||||
`1.0` when the cell's own `Cat` coordinate is `Item` and `0.0` otherwise. It
|
||||
keeps the evaluator `f64`-only — no string comparison enters `eval_expr` — and
|
||||
it reuses the primitive `Formula::filter` already had, `context.get(&cat)`,
|
||||
including its `unwrap_or(false)`: a cell with **no** `Cat` coordinate is not
|
||||
that item. Under route-then-aggregate that arm is reached only where a record
|
||||
genuinely lacks the coordinate, because the fold hands the expression a record
|
||||
key. Evaluated bare, outside a `SUM`, it degrades exactly as `Formula::filter`
|
||||
does — which is why a fitted tree is always emitted wrapped.
|
||||
|
||||
@@ -43,12 +43,18 @@ Three entry points, because the callers do not want the same thing:
|
||||
|
||||
An item literally named `SUM` is written `|SUM|`. `is_bare_name` is
|
||||
deliberately **stricter than the grammar strictly requires**: bare keywords
|
||||
(`WHERE SUM AVG MIN MAX COUNT IF`) *do* parse today, but only because
|
||||
(`WHERE SUM AVG MIN MAX COUNT IF IS`) *do* parse today, but only because
|
||||
`agg_call` needs a `(` and PEG backtracking then falls through to `ref_expr`
|
||||
— an emergent property of **rule ordering**, not a stated rule. A pipe-quoted
|
||||
name is an identifier in every position, with nothing to fall through, so a
|
||||
name colliding with a keyword is quoted and stops depending on that ordering.
|
||||
|
||||
`IS` joined the list with `is_expr` (improvise-4zl7), and it is the one keyword
|
||||
whose omission would have been *silently* wrong rather than merely fragile: a
|
||||
category literally named `IS` written bare turns `IS IS East` into a predicate
|
||||
on a category called `East`. Adding a production to `formula.pest` means adding
|
||||
its keyword here — the two are one change.
|
||||
|
||||
The cost is worth paying because a `## Formulas` line carries formula text
|
||||
**verbatim**: a name `format_md` writes is later read by `parse_formula`. One
|
||||
namespace, one spelling rule.
|
||||
@@ -65,6 +71,27 @@ io returned it and `parser.rs` panicked; the survivor returns it.
|
||||
|
||||
---
|
||||
|
||||
## `Cat IS Item`: a coordinate predicate in the expression grammar
|
||||
|
||||
`is_expr = { identifier ~ ^"IS" ~ filter_value }` sits in `primary` **before**
|
||||
`ref_expr`, which would otherwise consume the category name and leave `IS Item`
|
||||
unparseable. A bare name still parses as a reference: `is_expr` needs the
|
||||
keyword, fails without it, and PEG backtracks into `ref_expr`.
|
||||
|
||||
It reuses `where_clause`'s two operands — an `identifier` for the category and a
|
||||
`filter_value` for the item — and therefore `build_filter`, which already reads
|
||||
exactly that pair. The `^"IS"` literal is a string in the grammar rather than a
|
||||
rule, so it contributes no pair and `into_inner()` yields the two operands and
|
||||
nothing else. One builder, one quoting rule, both halves through
|
||||
`improvise_formula::quote`.
|
||||
|
||||
The grammar-walking generator needs no new case: it reads `formula.pest` through
|
||||
`pest_meta` at test time, so a production added to `primary` is generated and
|
||||
round-tripped from the day it exists.
|
||||
|
||||
What the *evaluator* does with it, and why a fitted tree is always wrapped in a
|
||||
`SUM`, is `context/code/core.md` §13.
|
||||
|
||||
## A formula target is unquoted, exactly as a reference is
|
||||
|
||||
`parse_formula` strips the pipes off the target the same way it does off an
|
||||
|
||||
+99
-67
@@ -209,14 +209,22 @@ A regression tree *is* a nested conditional, and improvise formulas already have
|
||||
an ordinary formula with nothing GP-specific left in the file:
|
||||
|
||||
```text
|
||||
Score = IF(humidity < 77.5, 1.25, IF(|outlook=rainy| < 0.5, -0.5, 2))
|
||||
Score = SUM(IF(humidity < 77.5, 1.25, IF(outlook IS rainy < 0.5, -0.5, 2)))
|
||||
```
|
||||
|
||||
(That sample used to sit in `improvise-gp/src/lib.rs`'s module doc inside a
|
||||
```text fence. rustdoc does not execute a `text` fence, so it was prose wearing
|
||||
a fence rather than a doctest, and the two-line cap applies to it like anything
|
||||
else. Note it is also an example of a formula `fitted_formula` would **refuse**,
|
||||
`outlook=rainy` being an encoding artefact — see §11.)
|
||||
(An earlier form of that sample sat in `improvise-gp/src/lib.rs`'s module doc
|
||||
inside a ```text fence. rustdoc does not execute a `text` fence, so it was prose
|
||||
wearing a fence rather than a doctest, and the two-line cap applies to it like
|
||||
anything else.)
|
||||
|
||||
Two things in it are improvise-4zl7's, and both are load-bearing. The outer
|
||||
`SUM` is the **route-then-aggregate rule made syntax**: a tree is fitted on
|
||||
records, and `SUM(e)` is improvise's existing word for "fold `e` over the member
|
||||
records" (`context/code/core.md` §13). Without it the formula routes on the
|
||||
totals one row up and means something else there; with it, a record row is a
|
||||
one-member fold and answers the rule itself. `outlook IS rainy` is the one-hot
|
||||
`outlook=rainy` said as a **coordinate predicate** rather than as a reference to
|
||||
an item no document has — see §11 for which features get which spelling.
|
||||
|
||||
Two properties the emitter owes its caller:
|
||||
|
||||
@@ -258,58 +266,74 @@ items of the document**, and `Expr::Ref` resolves through
|
||||
`Model::find_item_category` — so a tree splitting on one emitted a perfectly
|
||||
well-formed formula whose every cell then evaluated to
|
||||
`Error("ref:Region=North")`, with nothing between the fit and the file to say so
|
||||
(improvise-8xhy).
|
||||
(improvise-8xhy). improvise-4zl7 closed the categorical third of that by giving
|
||||
the emitter a vocabulary for a coordinate, and by giving the *evaluator* a rule
|
||||
that makes such a formula mean the same thing at every level of aggregation
|
||||
(`context/code/core.md` §13).
|
||||
|
||||
`fitted_formula` therefore takes the `&Model` and refuses, listing what it could
|
||||
not resolve. `formula_text` still spells it, being the tree's own text.
|
||||
### `Features`: one entry per feature index, read off the plan
|
||||
|
||||
**The check asks the document, never the name's shape.** A measure literally
|
||||
named `Region=North` is a legal document and is accepted; the two are
|
||||
indistinguishable as strings, and only the document can tell them apart. Asking
|
||||
the real precondition also covers `.missing`, the date parts, and every future
|
||||
sub-feature naming scheme with no second copy of `preprocess`'s expansion rules
|
||||
to drift from.
|
||||
`Features::of(&Dataset, categories)` walks `gp_tree::data::feature_roles` — the
|
||||
**one** statement of `preprocess`'s expansion order, which `encode_row` drives
|
||||
off too — and pairs each role with the name `preprocess` gave it. A tree's
|
||||
`feat` indexes `Features` directly, so nothing anywhere reconstructs
|
||||
index → role from the feature *names*. `Spec::names` is `&[&str]` and could not
|
||||
have carried this; the earlier check matched names against the raw plan instead
|
||||
and said in this section that it was avoiding a second implementation of the
|
||||
expansion. Now there is no second implementation to avoid.
|
||||
|
||||
**This is an interim, and deliberately the smallest of three remedies.**
|
||||
Materialising the one-hots as real `_Measure` items pollutes the document with
|
||||
encoding artefacts the user never asked for. Teaching the emitter improvise's
|
||||
`WHERE` vocabulary needs a *scalar coordinate predicate* the formula language
|
||||
does not have — `Filter` gates a formula or an aggregate, it is not an `Expr` —
|
||||
so it needs a new `Expr` variant, a grammar production, and an answer to what
|
||||
the predicate means in a cell that aggregates over the category; and it would
|
||||
close only the categorical third, leaving `.missing` and the date parts exactly
|
||||
here. That is improvise-4zl7.
|
||||
Each feature gets one of three `Spelling`s, decided **by role and by column**,
|
||||
never by the shape of the name:
|
||||
|
||||
### The hole a name cannot close
|
||||
| role | column | spelling |
|
||||
|---|---|---|
|
||||
| `Numeric` | a measure | `Expr::Ref(measure)` |
|
||||
| `OneHot { value }` | a category, `value != "(missing)"` | `Cat IS Item` |
|
||||
| anything else | | `Artefact` |
|
||||
|
||||
`fitted_formula`'s question is "can this be spelled", and that is the right
|
||||
question for *that* check and wrong as the only check. A numeric category called
|
||||
`Cost` produces a feature called `Cost`; if a `_Measure` item is also called
|
||||
`Cost`, the emitted `Expr::Ref("Cost")` resolves cleanly — **to the measure**,
|
||||
which is not the column the tree was fitted on.
|
||||
The two exclusions are not fussiness. A one-hot over a **measure** column names
|
||||
no coordinate at all — the record's value for that measure is text, and
|
||||
`Cat IS Item` asks about coordinates. `(missing)` is the *absence* of a
|
||||
coordinate, which `Expr::Is` reads as `0` by §13's `unwrap_or(false)`, so the
|
||||
one spelling that would be wrong is exactly the one omitted. A `Numeric` role
|
||||
over a **category** column is the trap the previous version of this section
|
||||
called "the hole a name cannot close": a category whose items all parse as
|
||||
numbers produces a feature named after the category, and `Expr::Ref` would
|
||||
resolve it to a `_Measure` item of the same name — a different column. Both
|
||||
halves now fall out of `is_category`, which is a lookup in the header's own
|
||||
first `category_columns` entries.
|
||||
|
||||
`fit::only_raw_measure_features` closes it by asking `preprocess`'s **plan**
|
||||
instead, and **by name, never by position**. `Dataset::columns` is *not*
|
||||
index-aligned with the header: `preprocess` `continue`s past an entirely-missing
|
||||
column without pushing a `ColumnSpec`, so one empty category shifts every later
|
||||
spec left and a positional `i >= category_columns` test reads a numeric measure
|
||||
as a category and refuses a perfectly good fit.
|
||||
Duplicate header names are still refused outright, by `no_ambiguous_column_names`
|
||||
and before anything else: `is_category` decides by name, so a name that means two
|
||||
columns is a spelling with two meanings.
|
||||
|
||||
The header that `feature_table` built is the reliable statement of role: its
|
||||
first `category_columns` entries are `Model::regular_category_names`, the rest up
|
||||
to the target are measures. A used feature is accepted only when its name is a
|
||||
**measure** column of the header *and* the plan gives that column kind
|
||||
`Numeric` — which rules out every one-hot, every `.missing` flag, every date
|
||||
part and every category coordinate, by role and not by spelling. Duplicate
|
||||
header names are refused outright first, since a name that means two columns
|
||||
cannot be resolved by either check.
|
||||
`Dataset::columns` is **not** index-aligned with the header — `preprocess`
|
||||
`continue`s past an entirely-missing column without pushing a `ColumnSpec`, so
|
||||
one empty category shifts every later spec left. `FeatureRole::column()` indexes
|
||||
`columns`, not the header, so that shift is carried rather than tripped over.
|
||||
|
||||
It deliberately does **not** reconstruct the expansion order index by index:
|
||||
that is improvise-4zl7's job and its ticket warns against a second
|
||||
implementation of it. Matching a used feature's name against the raw plan is
|
||||
sound in the *refusing* direction — `preprocess` names a plain numeric column
|
||||
exactly `col` and gives every expanded feature a suffix or an infix — and a
|
||||
false refusal costs a fit, where a false acceptance costs a wrong rule.
|
||||
### What `fitted_formula` still asks the document
|
||||
|
||||
Two refusals, in order. An `Artefact` is refused with no reference to the
|
||||
document at all — nothing could make `Cost.missing` spellable. Everything else
|
||||
is asked **of the document**: a `Measure` must be `find_item_category`-resolvable
|
||||
and a `Coordinate` must name a category the model has. A measure literally named
|
||||
`Region=North` is a legal document and is accepted, because the check never looks
|
||||
at the name's shape — and that is also why the date parts and the `.missing`
|
||||
flags stay covered without a second copy of `preprocess`'s naming rules.
|
||||
|
||||
`formula_text` still spells whatever it is handed, being the tree's own text; an
|
||||
`Artefact` reaching it is a precondition failure and panics, the same split
|
||||
`number_text` makes for a non-finite value.
|
||||
|
||||
### The two thirds this still does not close
|
||||
|
||||
`col.missing` means "this record has no value for measure X" — a different
|
||||
vocabulary, and one the formula language has no word for at all. The date
|
||||
sub-features (`when.year`, `when.month`) need date arithmetic improvise does not
|
||||
have. Both stay `Artefact`, both stay refused, and both are filed as
|
||||
improvise-wd6n rather than smuggled in — separately from each other, because
|
||||
they need different vocabulary.
|
||||
|
||||
## 12. A fit is persisted as its formula, and nothing else (improvise-7na)
|
||||
|
||||
@@ -496,21 +520,25 @@ bar. Flattening only the success path left the failure path able to split it.
|
||||
A related trap with no mechanism guarding it: a `rustfmt` pass over a multi-line
|
||||
string literal leaves runs of ten spaces mid-sentence in a message a user
|
||||
actually sees — invisible in the source and in every test that only asks whether
|
||||
a substring is present. `a_fit_that_needs_a_category_is_refused_naming_the_feature`
|
||||
asserts no `\n` and no double space for that reason.
|
||||
a substring is present. `a_name_containing_a_newline_cannot_split_the_status_line`
|
||||
asserts the one-line rule for that reason.
|
||||
|
||||
## 15. Known gaps, filed rather than fixed
|
||||
|
||||
**Aggregate-then-route is not route-then-aggregate (improvise-4zl7).** A tree is
|
||||
fitted on individual **records**. An improvise formula is evaluated at a
|
||||
`CellKey`, and in any cell that aggregates over a hidden category
|
||||
`evaluate_aggregated` aggregates each `Expr::Ref` *first* and then evaluates the
|
||||
`IF`. For a linear formula that is the same answer — `SUM(Revenue) - SUM(Cost)`
|
||||
is `SUM(Revenue - Cost)` — and for a tree it is not. **So the stored rule means
|
||||
one thing on a record cell and a different thing one row up.** The user has
|
||||
ruled that the fix is to aggregate the rule over the members; until then
|
||||
`a_fitted_rule_at_an_aggregated_cell_routes_on_the_aggregate_not_the_records`
|
||||
characterises it rather than asserting it is right.
|
||||
**Closed: aggregate-then-route is not route-then-aggregate (improvise-4zl7).**
|
||||
Kept here because the shape recurs. A tree is fitted on individual **records**,
|
||||
and an improvise formula evaluated at an aggregating `CellKey` used to aggregate
|
||||
each `Expr::Ref` first and evaluate the `IF` once on the totals — the same answer
|
||||
for a linear formula, a different one for anything that routes, so the stored
|
||||
rule meant one thing on a record cell and another one row up. The emitter now
|
||||
wraps every tree in `SUM(…)` and `eval_agg` folds a non-`Ref` inner expression
|
||||
over the member records: `context/code/core.md` §13. What remains of it is a
|
||||
**cost**, not a defect — the fold is O(members) per evaluation per fixed-point
|
||||
pass and is unbenchmarked (improvise-9iyv).
|
||||
|
||||
**Two artefact families are still unspellable (improvise-wd6n).** `col.missing`
|
||||
and the date sub-features stay `Spelling::Artefact` and are still refused. §11
|
||||
says what each would need; they are independent of each other.
|
||||
|
||||
**A mean-imputed feature is read raw by the formula.** `preprocess` fills a
|
||||
missing numeric cell with the column mean, so a champion may split on a value
|
||||
@@ -544,12 +572,16 @@ measures too** — the only shape `fit_measure` currently stores — so "the cas
|
||||
that stores leaks nothing" is wrong. Closing it means building the ranges and
|
||||
the preprocessing plan per training fold and encoding each validation fold with
|
||||
*that* plan, which `cv_regress` cannot express: it takes one spec and rows that
|
||||
are already encoded. That is an API change in `gp-tree`, and it is the same
|
||||
change improvise-4zl7 will want, so the two are filed to land together.
|
||||
are already encoded. That is an API change in `gp-tree`, and it is
|
||||
independent of improvise-4zl7, which turned out to need no change to how
|
||||
`cv_regress` is fed.
|
||||
|
||||
**Which features a champion picks is a property of the search**, so on a
|
||||
document with several categories the same command refuses or succeeds depending
|
||||
on the seed.
|
||||
**Which features a champion picks is a property of the search**, which used to
|
||||
mean the same command refused or succeeded depending on the seed. For the
|
||||
categorical family that is gone: a one-hot over a real category is spellable
|
||||
whichever way the search goes. It survives for `col.missing` and the date parts
|
||||
(improvise-wd6n), and it is the reason those are worth closing rather than
|
||||
living with — a refusal that depends on a seed is not a contract.
|
||||
|
||||
## 16. `preprocess`: what each column kind expands into
|
||||
|
||||
|
||||
@@ -411,3 +411,69 @@ with the entry above this pins the rotation without enumerating it twice.
|
||||
|
||||
Distinct axes must be distinguishable at a glance; `color()` is the only reason
|
||||
`AxisColor` exists, so a collision defeats it.
|
||||
|
||||
### a_routing_formula_over_a_group_sums_the_members_answers
|
||||
|
||||
improvise-4zl7, and the hand-computed half of its evidence. Every number in
|
||||
`route_then_aggregate_tests` is written out rather than obtained from a second
|
||||
call into the evaluator, because the bug being guarded is *the evaluator
|
||||
answering consistently and wrongly*.
|
||||
|
||||
The fixture is four records — `Cost` of 10 and 100 in North, 20 and 200 in
|
||||
South — and the formula is `Pred = SUM(IF(Cost < 50, 1, 10))`. `Cost` straddles
|
||||
the threshold **inside each region**, which is the whole design of the fixture:
|
||||
|
||||
| cell | route-then-aggregate | aggregate-then-route |
|
||||
|---|---|---|
|
||||
| one record (`Cost` 10) | 1 | 1 |
|
||||
| North (`Tag` away) | 1 + 10 = **11** | `110 < 50` → **10** |
|
||||
| everything | 1+10+1+10 = **22** | `330 < 50` → **10** |
|
||||
|
||||
A fixture whose records fall on the same side of the split would give the same
|
||||
answer either way at the record level and differ only by a factor of the record
|
||||
count, which is the kind of agreement that reads as correctness.
|
||||
|
||||
Two faults were injected and both were watched to fail. Restoring the old
|
||||
`eval_agg` — sum the matching raw values and ignore the inner expression —
|
||||
fails **7 of the 8** tests with 330 for 22, 110 for 11 and 10 for 1; the one
|
||||
survivor is `is_is_one_when_the_cells_own_coordinate_matches_and_zero_otherwise`,
|
||||
which has no `SUM` and so never reaches the fold. Making `eval_is` return `0.0`
|
||||
unconditionally fails exactly the **3** coordinate tests, with 400 for 214 and
|
||||
200 for 14. The two faults failing disjoint-but-overlapping sets is itself the
|
||||
evidence that the tests separate the fold from the predicate.
|
||||
|
||||
### a_coordinate_split_survives_aggregating_over_that_very_category
|
||||
|
||||
The case the ruling exists for, and the one the ticket's own text called the
|
||||
product question. With `Region` aggregated away the cell has no `Region`
|
||||
coordinate of its own, so a formula that read the *cell's* coordinate would see
|
||||
nothing and take `Expr::Is`'s `unwrap_or(false)` branch — answering 100 once.
|
||||
The fold hands the expression a **record** key, so each of the four records
|
||||
answers for its own coordinate: 7 + 7 + 100 + 100 = 214.
|
||||
|
||||
### a_member_whose_input_is_missing_errors_rather_than_being_dropped
|
||||
|
||||
The asymmetry between the `Expr::Ref` fast path and the fold, asserted rather
|
||||
than left to be discovered. `SUM(Revenue)` skips a record with no `Revenue`
|
||||
cell; `SUM(<expr>)` cannot, because the expression is what is being asked and an
|
||||
expression that will not evaluate is an error. The fixture clears one record's
|
||||
`Cost` and writes a different measure on the same record, so the record still
|
||||
exists and still has a member cell to be found by — otherwise the record would
|
||||
simply vanish from `matching_cells` and the test would prove nothing.
|
||||
|
||||
### feature_roles_line_up_with_the_names_preprocess_emitted
|
||||
|
||||
Lives in `gp-tree`, recorded here because the rule it guards is improvise-4zl7's.
|
||||
`feature_roles` is the one statement of `preprocess`'s expansion order and
|
||||
`encode_row` drives off it, so a drift is silent in both directions: the emitter
|
||||
would spell the wrong feature and a re-encoded row would land in the wrong
|
||||
column. The assertion rebuilds each feature's *name* from its role and compares
|
||||
it with the name `preprocess` emitted, which is the only thing that ties the two
|
||||
walks together.
|
||||
|
||||
The last four assertions exist because the loop is vacuous over a fixture that
|
||||
produces only one kind of feature. The fixture has a numeric column with a
|
||||
missing cell, a categorical column and a date column, so all four roles appear.
|
||||
Fault injected: swapping the `Numeric` and `Missing` pushes in `feature_roles`
|
||||
fails this test **and** `encode_row_replays_the_training_expansion`, which is
|
||||
the pair that would have disagreed.
|
||||
|
||||
+70
-39
@@ -271,13 +271,27 @@ equals the threshold routes `hi` under `eval` and would route `lo` through a
|
||||
formula carrying the rounded threshold. The test asserts the trap is live —
|
||||
`"0.7".parse::<f64>() > thresh as f64` — before relying on it.
|
||||
|
||||
### a_split_on_a_one_hot_feature_is_refused
|
||||
### a_split_on_a_one_hot_feature_is_spelled_as_a_coordinate
|
||||
|
||||
improvise-8xhy, the flip of the characterization test it replaces. `preprocess`
|
||||
improvise-8xhy's defect, and improvise-4zl7's answer to it. `preprocess`
|
||||
one-hots a categorical column into features named `Region=North`, an encoding
|
||||
artefact no document has an item for, so `find_item_category` returns `None` and
|
||||
every cell of the fitted measure came back as `Error("ref:Region=North")` —
|
||||
silently, with the emitter having said nothing.
|
||||
artefact no document has an item for, so `find_item_category` returned `None`
|
||||
and every cell of the fitted measure came back as `Error("ref:Region=North")` —
|
||||
silently, with the emitter having said nothing. 8xhy shipped a refusal; 4zl7
|
||||
replaced the refusal with a spelling, `Region IS North`.
|
||||
|
||||
The assertion is the emitted **text**, exactly, and not merely that the call is
|
||||
`Ok`: the whole failure mode here was a well-formed formula meaning the wrong
|
||||
thing, so "it parsed" is the assertion that missed it the first time. The
|
||||
fault to inject is `Spelling::Coordinate` rendering as `quote_name(name)` —
|
||||
byte-identical for every measure, and wrong for exactly this family.
|
||||
|
||||
### a_coordinate_naming_no_category_of_this_document_is_refused
|
||||
|
||||
The negative arm of the same rule, and it is not redundant with the artefact
|
||||
refusal: a `Coordinate` is *spellable* and may still be unresolvable. Without
|
||||
it, `resolves_in`'s `Coordinate` arm could return `true` unconditionally and
|
||||
every test above would still pass.
|
||||
|
||||
### a_measure_that_merely_looks_like_a_one_hot_is_accepted
|
||||
|
||||
@@ -419,12 +433,21 @@ the header once anything is dropped. A positional `i >= category_columns` role
|
||||
test then read the surviving numeric measure as a category and refused a
|
||||
perfectly good fit. The test asserts the fixture really drops a column first.
|
||||
|
||||
### a_category_coordinate_is_refused_even_when_an_item_shares_its_name
|
||||
### a_coordinate_is_spelled_as_one_even_when_an_item_shares_the_one_hots_name
|
||||
|
||||
The other half of the same rule, and the one a *name* cannot catch. The fixture
|
||||
adds a `_Measure` item literally named `Region=North` and asserts
|
||||
`find_item_category` resolves it, so the older check would have passed — which
|
||||
is what makes this test prove something the previous one does not.
|
||||
The collision a *name* cannot resolve, now closed by construction rather than by
|
||||
a refusal. The fixture adds a `_Measure` item literally named `Region=North` and
|
||||
asserts `find_item_category` resolves it — so a check that matched the one-hot's
|
||||
feature name against the document would have found the wrong column.
|
||||
|
||||
Under improvise-4zl7 the one-hot is never spelled by its feature name at all:
|
||||
`Features::of` reads the **role** off `preprocess`'s plan and emits
|
||||
`Region IS North`, which cannot collide with any measure however the document is
|
||||
named. So the assertion is the *absence* of `|Region=North|` in the emitted text
|
||||
as much as the presence of `Region IS`. Injecting the fault means having
|
||||
`spell` fall through to `Spelling::Measure` for a `OneHot` role: the fit then
|
||||
succeeds, the formula reads `|Region=North|`, and it silently evaluates against
|
||||
the wrong column — which is the original bug wearing the new API.
|
||||
|
||||
### a_high_cardinality_numeric_measure_is_one_feature_not_many
|
||||
|
||||
@@ -484,18 +507,23 @@ reaches the search.
|
||||
`.improv` pipe quoting makes a newline legal inside a name, so a document can
|
||||
hold a measure whose name would split the TUI status bar in two.
|
||||
|
||||
### a_fit_that_needs_a_category_is_refused_naming_the_feature
|
||||
### a_fit_that_needs_a_category_now_spells_it_as_a_coordinate
|
||||
|
||||
improvise-8xhy reaching a real user, and the reason improvise-4zl7 exists. The
|
||||
fixture clears **both** measures on every North record: clearing only `Cost`
|
||||
would leave North carrying both target values, so the one-hot would explain
|
||||
nothing and the champion would have no reason to split on it.
|
||||
improvise-8xhy reaching a real user, and what improvise-4zl7 bought: the same
|
||||
fixture, the same champion, and a formula instead of a refusal. The fixture
|
||||
clears **both** measures on every North record: clearing only `Cost` would leave
|
||||
North carrying both target values, so the one-hot would explain nothing and the
|
||||
champion would have no reason to split on it.
|
||||
|
||||
It also asserts the refusal contains no `\n` and no double space. A `rustfmt`
|
||||
pass over the multi-line string literal that spells the message had left runs of
|
||||
ten spaces mid-sentence — invisible in the source and in every test that only
|
||||
asked whether a substring was present, and visible to precisely the user the
|
||||
message exists for.
|
||||
The second assertion is the one that earns its place. Checking only that the
|
||||
text says `Region IS` would pass for a rule that routed both groups to the same
|
||||
leaf — which is what a tree that could not see the coordinate would produce. The
|
||||
fixture's two groups are 1000 and 4000, so requiring one leaf under 2000 and one
|
||||
over 3000 separates "spelled it" from "spelled it and meant it".
|
||||
|
||||
The `\n`-and-double-space assertions moved with the refusal they guarded; the
|
||||
one-line rule itself is unchanged and is still asserted by
|
||||
`a_name_containing_a_newline_cannot_split_the_status_line`.
|
||||
|
||||
### the_fold_count_moves_the_champion_and_so_is_recorded
|
||||
|
||||
@@ -524,25 +552,28 @@ item installed cleanly and then made every later formula referring to it read
|
||||
the category coordinate. `a_later_formula_referring_to_the_fit_reads_the_fit` is
|
||||
the property that refusal protects, asserted against a real document.
|
||||
|
||||
### a_fitted_rule_at_an_aggregated_cell_routes_on_the_aggregate_not_the_records
|
||||
### a_fitted_rule_at_an_aggregated_cell_routes_per_record_and_sums
|
||||
|
||||
**The most important finding of PR #25, and a product question rather than a
|
||||
defect.** A tree is fitted on individual records; an improvise formula is
|
||||
evaluated at a `CellKey`, and in any cell that aggregates over a hidden category
|
||||
`evaluate_aggregated` aggregates each `Expr::Ref` *first* and then evaluates the
|
||||
`IF`. Linear formulas are unaffected — `SUM(Revenue) - SUM(Cost)` is
|
||||
`SUM(Revenue - Cost)` — and a tree is not. So the stored rule means one thing on
|
||||
a record cell and a different thing one row up.
|
||||
The characterization PR #25 left here, now replaced by the rule it was waiting
|
||||
for. A tree is fitted on individual records; an improvise formula is evaluated
|
||||
at a `CellKey`, and `evaluate_aggregated` used to aggregate each `Expr::Ref`
|
||||
*first* and evaluate the `IF` once on the totals. Linear formulas are unaffected
|
||||
— `SUM(Revenue) - SUM(Cost)` is `SUM(Revenue - Cost)` — and a tree is not, so
|
||||
the stored rule meant one thing on a record cell and a different thing one row
|
||||
up. improvise-4zl7's ruling was to aggregate the rule over the members.
|
||||
|
||||
This **characterises** rather than asserts correctness: what to do about it is
|
||||
improvise-4zl7, now re-scoped, and the user has ruled that the fix is to
|
||||
aggregate the rule over the members.
|
||||
The old test asserted the **negative** and carried a message telling whoever
|
||||
broke it to replace it with the rule. It fired on the first run of the fold.
|
||||
|
||||
Two things about the assertion. It asserts the **negative** — that the
|
||||
aggregated cell does *not* agree with the sum of the per-record predictions —
|
||||
which covers both outcomes and fails the day aggregation becomes record-wise,
|
||||
which is the day the characterization should be replaced by the rule. And on
|
||||
this document the aggregated cell is in fact `Error("ref:Cost")` rather than a
|
||||
wrongly-routed number, which is the loud failure this repo prefers and is **not
|
||||
something to rely on**: it is what this shape happens to produce, not a rule
|
||||
anything states.
|
||||
What this one asserts is the property the ticket names: the aggregated cell
|
||||
equals the sum of the per-record predictions. That compares two *different*
|
||||
code paths — a fully-specified key evaluates one member, an aggregating key
|
||||
folds twelve — rather than one path against itself, but it is still an
|
||||
end-to-end agreement rather than a hand-computed number. **The hand-computed
|
||||
evidence is in `improvise-core`**, under
|
||||
`route_then_aggregate_tests`, on a four-record fixture whose every expected
|
||||
value is written out in the test name or the assertion.
|
||||
|
||||
The third assertion is what stops this one being satisfiable by accident:
|
||||
routing once on the total lands on a single leaf, so the test refuses a fixture
|
||||
in which a leaf happens to equal the right answer.
|
||||
|
||||
+149
-33
@@ -452,32 +452,55 @@ pub fn preprocess(header: &[String], rows: &[Vec<String>], ov: &Overrides) -> Da
|
||||
}
|
||||
}
|
||||
|
||||
/// Re-encode one raw row into the layout `columns` expects — the inverse of
|
||||
/// [`preprocess`]'s expansion, driven by the saved plan rather than re-inferring.
|
||||
pub fn encode_row(columns: &[ColumnSpec], header: &[String], row: &[String]) -> Vec<f32> {
|
||||
let mut out = Vec::new();
|
||||
for col in columns {
|
||||
let j = header
|
||||
.iter()
|
||||
.position(|h| h == &col.name)
|
||||
.unwrap_or_else(|| panic!("the row is missing column {:?}", col.name));
|
||||
let cell = &row[j];
|
||||
let missing = is_missing(cell);
|
||||
/// One expanded feature's job: the raw column it reads, and what it reads off that
|
||||
/// column's cell. Roles carry their own imputation — `context/code/gp.md` §16.
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub enum FeatureRole {
|
||||
Numeric {
|
||||
column: usize,
|
||||
impute: f32,
|
||||
},
|
||||
DatePart {
|
||||
column: usize,
|
||||
part: usize,
|
||||
impute: f32,
|
||||
},
|
||||
Missing {
|
||||
column: usize,
|
||||
},
|
||||
OneHot {
|
||||
column: usize,
|
||||
value: String,
|
||||
},
|
||||
}
|
||||
|
||||
impl FeatureRole {
|
||||
/// Which entry of the `columns` plan this feature was expanded from.
|
||||
pub fn column(&self) -> usize {
|
||||
match self {
|
||||
Self::Numeric { column, .. }
|
||||
| Self::DatePart { column, .. }
|
||||
| Self::Missing { column }
|
||||
| Self::OneHot { column, .. } => *column,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Every feature a saved plan expands to, in the order [`preprocess`] emitted them.
|
||||
/// The **one** statement of that order: [`encode_row`] drives off it too.
|
||||
pub fn feature_roles(columns: &[ColumnSpec]) -> Vec<FeatureRole> {
|
||||
let mut roles = Vec::new();
|
||||
for (column, col) in columns.iter().enumerate() {
|
||||
match &col.kind {
|
||||
ColumnKind::Numeric {
|
||||
impute,
|
||||
has_missing,
|
||||
} => {
|
||||
out.push(if missing {
|
||||
*impute
|
||||
} else {
|
||||
cell.parse().unwrap_or_else(|_| {
|
||||
panic!("column {:?}: {cell:?} is not numeric", col.name)
|
||||
})
|
||||
roles.push(FeatureRole::Numeric {
|
||||
column,
|
||||
impute: *impute,
|
||||
});
|
||||
if *has_missing {
|
||||
out.push(missing as i32 as f32);
|
||||
}
|
||||
roles.extend(has_missing.then_some(FeatureRole::Missing { column }));
|
||||
}
|
||||
ColumnKind::Date {
|
||||
has_time,
|
||||
@@ -485,24 +508,67 @@ pub fn encode_row(columns: &[ColumnSpec], header: &[String], row: &[String]) ->
|
||||
has_missing,
|
||||
} => {
|
||||
let width = if *has_time { 7 } else { 5 };
|
||||
if missing {
|
||||
out.extend_from_slice(&impute[..width]);
|
||||
} else {
|
||||
let dt = parse_datetime(cell)
|
||||
.unwrap_or_else(|| panic!("column {:?}: {cell:?} is not a date", col.name));
|
||||
out.extend_from_slice(&date_parts(&dt)[..width]);
|
||||
}
|
||||
if *has_missing {
|
||||
out.push(missing as i32 as f32);
|
||||
}
|
||||
roles.extend((0..width).map(|part| FeatureRole::DatePart {
|
||||
column,
|
||||
part,
|
||||
impute: impute[part],
|
||||
}));
|
||||
roles.extend(has_missing.then_some(FeatureRole::Missing { column }));
|
||||
}
|
||||
ColumnKind::Categorical { values } => {
|
||||
let key = if missing { MISSING_CAT } else { cell.as_str() };
|
||||
out.extend(values.iter().map(|v| (key == v) as i32 as f32));
|
||||
roles.extend(values.iter().map(|value| FeatureRole::OneHot {
|
||||
column,
|
||||
value: value.clone(),
|
||||
}));
|
||||
}
|
||||
}
|
||||
}
|
||||
out
|
||||
roles
|
||||
}
|
||||
|
||||
/// One feature's value for one raw cell. The role carries everything the reading
|
||||
/// needs, so there is no arm here a plan cannot reach.
|
||||
fn encode_feature(role: &FeatureRole, cell: &str, name: &str) -> f32 {
|
||||
let missing = is_missing(cell);
|
||||
match role {
|
||||
FeatureRole::Numeric { impute, .. } => match missing {
|
||||
true => *impute,
|
||||
false => cell
|
||||
.parse()
|
||||
.unwrap_or_else(|_| panic!("column {name:?}: {cell:?} is not numeric")),
|
||||
},
|
||||
FeatureRole::DatePart { part, impute, .. } => match missing {
|
||||
true => *impute,
|
||||
false => {
|
||||
let dt = parse_datetime(cell)
|
||||
.unwrap_or_else(|| panic!("column {name:?}: {cell:?} is not a date"));
|
||||
date_parts(&dt)[*part]
|
||||
}
|
||||
},
|
||||
FeatureRole::Missing { .. } => missing as i32 as f32,
|
||||
FeatureRole::OneHot { value, .. } => {
|
||||
(if missing { MISSING_CAT } else { cell } == value.as_str()) as i32 as f32
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Re-encode one raw row into the layout `columns` expects — the inverse of
|
||||
/// [`preprocess`]'s expansion, driven by the saved plan rather than re-inferring.
|
||||
pub fn encode_row(columns: &[ColumnSpec], header: &[String], row: &[String]) -> Vec<f32> {
|
||||
let cells: Vec<&str> = columns
|
||||
.iter()
|
||||
.map(|col| {
|
||||
let j = header
|
||||
.iter()
|
||||
.position(|h| h == &col.name)
|
||||
.unwrap_or_else(|| panic!("the row is missing column {:?}", col.name));
|
||||
row[j].as_str()
|
||||
})
|
||||
.collect();
|
||||
feature_roles(columns)
|
||||
.iter()
|
||||
.map(|role| encode_feature(role, cells[role.column()], &columns[role.column()].name))
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -831,6 +897,56 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// `feature_roles` is the *one* statement of the expansion order (improvise-4zl7),
|
||||
/// so it must line up with the names `preprocess` emitted, entry for entry.
|
||||
#[test]
|
||||
fn feature_roles_line_up_with_the_names_preprocess_emitted() {
|
||||
let (h, rows) = table(
|
||||
&["size", "region", "when", "price"],
|
||||
&[
|
||||
&["1", "east", "2024-01-02", "10"],
|
||||
&["", "west", "2024-06-30", "20"],
|
||||
&["3", "east", "", "30"],
|
||||
],
|
||||
);
|
||||
let d = preprocess(&h, &rows, &Overrides::default());
|
||||
let roles = feature_roles(&d.columns);
|
||||
assert_eq!(roles.len(), d.feature_names.len(), "{:?}", d.feature_names);
|
||||
// Each role must name the column its feature name is built from, and say
|
||||
// which of the four kinds of feature it is.
|
||||
for (role, name) in roles.iter().zip(&d.feature_names) {
|
||||
let column = &d.columns[role.column()].name;
|
||||
let expected = match role {
|
||||
FeatureRole::Numeric { .. } => column.clone(),
|
||||
FeatureRole::Missing { .. } => format!("{column}.missing"),
|
||||
FeatureRole::DatePart { part, .. } => format!("{column}.{}", DATE_PARTS[*part]),
|
||||
FeatureRole::OneHot { value, .. } => format!("{column}={value}"),
|
||||
};
|
||||
assert_eq!(&expected, name, "{roles:?}");
|
||||
}
|
||||
// And the fixture really exercises all four, or the loop proves little.
|
||||
assert!(
|
||||
roles
|
||||
.iter()
|
||||
.any(|r| matches!(r, FeatureRole::Numeric { .. }))
|
||||
);
|
||||
assert!(
|
||||
roles
|
||||
.iter()
|
||||
.any(|r| matches!(r, FeatureRole::Missing { .. }))
|
||||
);
|
||||
assert!(
|
||||
roles
|
||||
.iter()
|
||||
.any(|r| matches!(r, FeatureRole::DatePart { .. }))
|
||||
);
|
||||
assert!(
|
||||
roles
|
||||
.iter()
|
||||
.any(|r| matches!(r, FeatureRole::OneHot { .. }))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_missing_categorical_cell_becomes_its_own_level() {
|
||||
let (h, rows) = table(
|
||||
|
||||
@@ -100,8 +100,8 @@ mod tests {
|
||||
const REGION: &str = "Region";
|
||||
const TAG: &str = "Tag";
|
||||
|
||||
/// A document a tree can learn exactly. **Every category is a `Label`**, or the
|
||||
/// champion can reach an unresolvable feature — see the evidence.
|
||||
/// A document a tree can learn exactly. **Every category is a `Label`**, so
|
||||
/// `Cost` is the only thing the search can reach — see the evidence.
|
||||
fn workbook() -> crate::workbook::Workbook {
|
||||
let mut wb = crate::workbook::Workbook::new("sales");
|
||||
wb.add_category(REGION).expect("first category");
|
||||
|
||||
@@ -41,6 +41,16 @@ fn aggregate_cell_values(values: &[&CellValue], func: &AggFunc) -> Option<CellVa
|
||||
})
|
||||
}
|
||||
|
||||
/// The member records behind `partial`, in `DataStore` insertion order and
|
||||
/// without repeats — the coordinate tuples minus `_Measure` (§13).
|
||||
fn member_records(model: &Model, partial: &CellKey) -> Vec<CellKey> {
|
||||
let mut seen: IndexSet<CellKey> = IndexSet::new();
|
||||
for (key, _) in model.data.matching_cells(&partial.0) {
|
||||
seen.insert(key.without(MEASURE_CATEGORY));
|
||||
}
|
||||
seen.into_iter().collect()
|
||||
}
|
||||
|
||||
/// A formula-level aggregate over the data matching `context` minus the
|
||||
/// target. Deliberately *not* [`apply_agg`]'s emptiness rule — §2.
|
||||
fn eval_agg(
|
||||
@@ -50,6 +60,7 @@ fn eval_agg(
|
||||
func: &AggFunc,
|
||||
inner: &Expr,
|
||||
agg_filter: Option<&Filter>,
|
||||
eval_at: &dyn Fn(&Expr, &CellKey) -> Result<f64, String>,
|
||||
) -> Result<f64, String> {
|
||||
let mut partial = context.without(target_category);
|
||||
if let Expr::Ref(item_name) = inner
|
||||
@@ -60,12 +71,26 @@ fn eval_agg(
|
||||
if let Some(f) = agg_filter {
|
||||
partial = partial.with(&f.category, &f.item);
|
||||
}
|
||||
let values: Vec<f64> = model
|
||||
.data
|
||||
.matching_values(&partial.0)
|
||||
.into_iter()
|
||||
.filter_map(|v| v.as_f64())
|
||||
.collect();
|
||||
let values: Vec<f64> = match inner {
|
||||
// A bare ref asks for the values that exist; anything else asks the
|
||||
// expression of each member record and folds the answers — §13.
|
||||
Expr::Ref(_) => model
|
||||
.data
|
||||
.matching_values(&partial.0)
|
||||
.into_iter()
|
||||
.filter_map(|v| v.as_f64())
|
||||
.collect(),
|
||||
_ => member_records(model, &partial)
|
||||
.into_iter()
|
||||
.map(|record| {
|
||||
let at = record
|
||||
.0
|
||||
.iter()
|
||||
.fold(context.clone(), |k, (cat, item)| k.with(cat, item));
|
||||
eval_at(inner, &at)
|
||||
})
|
||||
.collect::<Result<Vec<f64>, String>>()?,
|
||||
};
|
||||
|
||||
match func {
|
||||
AggFunc::Sum => Ok(values.iter().sum()),
|
||||
@@ -76,6 +101,14 @@ fn eval_agg(
|
||||
}
|
||||
}
|
||||
|
||||
/// `Cat IS Item` as a number. A cell with no `Cat` coordinate is **not** that
|
||||
/// item, matching `Formula::filter`'s own `unwrap_or(false)` — §13.
|
||||
fn eval_is(context: &CellKey, filter: &Filter) -> f64 {
|
||||
context
|
||||
.get(&filter.category)
|
||||
.is_some_and(|v| v == filter.item.as_str()) as i32 as f64
|
||||
}
|
||||
|
||||
/// Pure-data document model: categories, cells, formulas. It knows nothing
|
||||
/// about views — see [`crate::workbook::Workbook`] and core.md §10.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
@@ -521,7 +554,9 @@ impl Model {
|
||||
func,
|
||||
inner.as_ref(),
|
||||
agg_filter.as_ref(),
|
||||
&|e, at| eval_expr_cached(e, at, model, target_category, none_cats),
|
||||
),
|
||||
Expr::Is(filter) => Ok(eval_is(context, filter)),
|
||||
Expr::If(cond, then, else_) => {
|
||||
let cv = eval_bool_cached(cond, context, model, target_category, none_cats)?;
|
||||
if cv {
|
||||
@@ -649,7 +684,9 @@ impl Model {
|
||||
func,
|
||||
inner.as_ref(),
|
||||
agg_filter.as_ref(),
|
||||
&|e, at| eval_expr(e, at, model, target_category, depth),
|
||||
),
|
||||
Expr::Is(filter) => Ok(eval_is(context, filter)),
|
||||
Expr::If(cond, then, else_) => {
|
||||
let cv = eval_bool(cond, context, model, target_category, depth)?;
|
||||
if cv {
|
||||
@@ -742,7 +779,8 @@ mod agg_helper_tests {
|
||||
let model = Model::new("T");
|
||||
let missing = Expr::Ref("Nonexistent".to_string());
|
||||
let ctx = CellKey::new(vec![]);
|
||||
let run = |func| eval_agg(&model, &ctx, "_Measure", &func, &missing, None);
|
||||
let never = |_: &Expr, _: &CellKey| unreachable!("a bare Ref never folds per record");
|
||||
let run = |func| eval_agg(&model, &ctx, "_Measure", &func, &missing, None, &never);
|
||||
|
||||
assert_eq!(run(AggFunc::Sum), Ok(0.0));
|
||||
assert_eq!(run(AggFunc::Count), Ok(0.0));
|
||||
@@ -2244,3 +2282,174 @@ mod prop_tests {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-4zl7: a routing formula is applied to each member record and the
|
||||
/// answers aggregated. Every number here is hand-computed — see the evidence file.
|
||||
#[cfg(test)]
|
||||
mod route_then_aggregate_tests {
|
||||
use super::*;
|
||||
use crate::formula::parse_formula;
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
|
||||
const MEASURE: &str = "_Measure";
|
||||
|
||||
fn key(pairs: &[(&str, &str)]) -> CellKey {
|
||||
CellKey::new(
|
||||
pairs
|
||||
.iter()
|
||||
.map(|(c, i)| ((*c).to_string(), (*i).to_string()))
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Four records over two regions, with `Cost` straddling 50 **inside** each
|
||||
/// region: 10 and 100 in North, 20 and 200 in South.
|
||||
fn four_records(formula: &str) -> Model {
|
||||
let mut m = Model::new("fit");
|
||||
for (cat, items) in [("Region", ["North", "South"]), ("Tag", ["a", "b"])] {
|
||||
m.add_category(cat).expect("under the limit");
|
||||
for item in items {
|
||||
m.category_mut(cat).expect("just added").add_item(item);
|
||||
}
|
||||
}
|
||||
m.category_mut(MEASURE)
|
||||
.expect("_Measure is always present")
|
||||
.add_item("Cost");
|
||||
for (tag, region, cost) in [
|
||||
("a", "North", 10.0),
|
||||
("b", "North", 100.0),
|
||||
("a", "South", 20.0),
|
||||
("b", "South", 200.0),
|
||||
] {
|
||||
m.set_cell(
|
||||
key(&[("Tag", tag), ("Region", region), (MEASURE, "Cost")]),
|
||||
CellValue::Number(cost),
|
||||
);
|
||||
}
|
||||
m.add_formula(parse_formula(formula, MEASURE).expect("the fixture parses"));
|
||||
m
|
||||
}
|
||||
|
||||
fn number_at(m: &Model, pairs: &[(&str, &str)]) -> f64 {
|
||||
match m.evaluate(&key(pairs)) {
|
||||
Some(CellValue::Number(n)) => n,
|
||||
other => panic!("{pairs:?} evaluated to {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
/// A record row: one member, so the fold is the rule itself. 10 < 50 and
|
||||
/// 100 is not, so the two North records answer 1 and 10.
|
||||
#[test]
|
||||
fn a_routing_formula_at_a_record_is_the_rule_itself() {
|
||||
let m = four_records("Pred = SUM(IF(Cost < 50, 1, 10))");
|
||||
assert_eq!(
|
||||
number_at(&m, &[("Tag", "a"), ("Region", "North"), (MEASURE, "Pred")]),
|
||||
1.0
|
||||
);
|
||||
assert_eq!(
|
||||
number_at(&m, &[("Tag", "b"), ("Region", "North"), (MEASURE, "Pred")]),
|
||||
10.0
|
||||
);
|
||||
}
|
||||
|
||||
/// One row up, `Tag` aggregated away: North is 1 + 10 = **11**. Routing on the
|
||||
/// aggregate would ask `110 < 50` once and answer **10**.
|
||||
#[test]
|
||||
fn a_routing_formula_over_a_group_sums_the_members_answers() {
|
||||
let m = four_records("Pred = SUM(IF(Cost < 50, 1, 10))");
|
||||
assert_eq!(
|
||||
number_at(&m, &[("Region", "North"), (MEASURE, "Pred")]),
|
||||
11.0
|
||||
);
|
||||
assert_eq!(
|
||||
number_at(&m, &[("Region", "South"), (MEASURE, "Pred")]),
|
||||
11.0
|
||||
);
|
||||
}
|
||||
|
||||
/// Everything aggregated away: 1 + 10 + 1 + 10 = **22**, where routing on the
|
||||
/// total would ask `330 < 50` once and answer **10**.
|
||||
#[test]
|
||||
fn a_routing_formula_over_the_whole_document_sums_every_member() {
|
||||
let m = four_records("Pred = SUM(IF(Cost < 50, 1, 10))");
|
||||
assert_eq!(number_at(&m, &[(MEASURE, "Pred")]), 22.0);
|
||||
}
|
||||
|
||||
/// The cached evaluator must answer the same. `recompute_formulas` is the
|
||||
/// fixed point of gotcha 9, and it caches the *aggregated* stems.
|
||||
#[test]
|
||||
fn the_cached_evaluator_routes_per_record_too() {
|
||||
let mut m = four_records("Pred = SUM(IF(Cost < 50, 1, 10))");
|
||||
let none_cats = ["Tag".to_string()];
|
||||
m.recompute_formulas(&none_cats);
|
||||
for region in ["North", "South"] {
|
||||
assert_eq!(
|
||||
m.evaluate_aggregated(&key(&[("Region", region), (MEASURE, "Pred")]), &none_cats),
|
||||
Some(CellValue::Number(11.0)),
|
||||
"{region}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A coordinate split: `Region IS North` is 1 on North's two records and 0 on
|
||||
/// South's, so `IF(… < 0.5, 100, 7)` gives 7+7 = **14** and 100+100 = **200**.
|
||||
#[test]
|
||||
fn a_coordinate_split_reads_each_records_own_coordinate() {
|
||||
let m = four_records("Pred = SUM(IF(Region IS North < 0.5, 100, 7))");
|
||||
assert_eq!(
|
||||
number_at(&m, &[("Region", "North"), (MEASURE, "Pred")]),
|
||||
14.0
|
||||
);
|
||||
assert_eq!(
|
||||
number_at(&m, &[("Region", "South"), (MEASURE, "Pred")]),
|
||||
200.0
|
||||
);
|
||||
}
|
||||
|
||||
/// The case the whole ruling exists for: with `Region` aggregated away the
|
||||
/// cell has no `Region` coordinate of its own, yet the answer is 14 + 200.
|
||||
#[test]
|
||||
fn a_coordinate_split_survives_aggregating_over_that_very_category() {
|
||||
let m = four_records("Pred = SUM(IF(Region IS North < 0.5, 100, 7))");
|
||||
assert_eq!(number_at(&m, &[(MEASURE, "Pred")]), 214.0);
|
||||
}
|
||||
|
||||
/// `Expr::Is` on its own: 1, 0, and 0 for a cell with no such coordinate —
|
||||
/// the same `unwrap_or(false)` a `WHERE` filter makes.
|
||||
#[test]
|
||||
fn is_is_one_when_the_cells_own_coordinate_matches_and_zero_otherwise() {
|
||||
let m = four_records("Pred = Region IS North");
|
||||
assert_eq!(
|
||||
number_at(&m, &[("Tag", "a"), ("Region", "North"), (MEASURE, "Pred")]),
|
||||
1.0
|
||||
);
|
||||
assert_eq!(
|
||||
number_at(&m, &[("Tag", "a"), ("Region", "South"), (MEASURE, "Pred")]),
|
||||
0.0
|
||||
);
|
||||
assert_eq!(number_at(&m, &[("Tag", "a"), (MEASURE, "Pred")]), 0.0);
|
||||
}
|
||||
|
||||
/// The fold asks the expression of each member, so a record whose input is
|
||||
/// missing is an error rather than a silently dropped row.
|
||||
#[test]
|
||||
fn a_member_whose_input_is_missing_errors_rather_than_being_dropped() {
|
||||
let mut m = four_records("Pred = SUM(IF(Cost < 50, 1, 10))");
|
||||
m.set_cell(
|
||||
key(&[("Tag", "a"), ("Region", "North"), (MEASURE, "Other")]),
|
||||
CellValue::Number(1.0),
|
||||
);
|
||||
m.clear_cell(&key(&[
|
||||
("Tag", "a"),
|
||||
("Region", "North"),
|
||||
(MEASURE, "Cost"),
|
||||
]));
|
||||
assert!(
|
||||
matches!(
|
||||
m.evaluate(&key(&[("Region", "North"), (MEASURE, "Pred")])),
|
||||
Some(CellValue::Error(_))
|
||||
),
|
||||
"a member with no Cost must surface, not vanish"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -40,6 +40,9 @@ pub enum Expr {
|
||||
UnaryMinus(Box<Expr>),
|
||||
Agg(AggFunc, Box<Expr>, Option<Filter>),
|
||||
If(Box<Expr>, Box<Expr>, Box<Expr>),
|
||||
/// `Cat IS Item` — 1.0 when the cell's own `Cat` coordinate is `Item`, else
|
||||
/// 0.0. A coordinate predicate as a number; `context/code/core.md` §13.
|
||||
Is(Filter),
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
|
||||
@@ -47,9 +47,14 @@ primary = {
|
||||
| agg_call
|
||||
| if_expr
|
||||
| paren_expr
|
||||
| is_expr
|
||||
| ref_expr
|
||||
}
|
||||
|
||||
// `Cat IS Item` — a coordinate predicate as 1/0. It must precede `ref_expr`,
|
||||
// which would otherwise consume the category name and leave `IS Item` behind.
|
||||
is_expr = { identifier ~ ^"IS" ~ filter_value }
|
||||
|
||||
paren_expr = { "(" ~ expr ~ ")" }
|
||||
|
||||
// Aggregates with optional inline WHERE filter inside the parens.
|
||||
|
||||
@@ -116,7 +116,7 @@ fn build_unary(pair: Pair<Rule>) -> Expr {
|
||||
}
|
||||
|
||||
fn build_primary(pair: Pair<Rule>) -> Expr {
|
||||
// primary = { number | agg_call | if_expr | paren_expr | ref_expr }
|
||||
// primary = { number | agg_call | if_expr | paren_expr | is_expr | ref_expr }
|
||||
let inner = first_inner(pair);
|
||||
let rule = inner.as_rule();
|
||||
if rule == Rule::number {
|
||||
@@ -127,6 +127,9 @@ fn build_primary(pair: Pair<Rule>) -> Expr {
|
||||
build_if_expr(inner)
|
||||
} else if rule == Rule::paren_expr {
|
||||
build_expr(first_inner(inner))
|
||||
} else if rule == Rule::is_expr {
|
||||
// Same shape as a WHERE clause minus the keyword: identifier ~ filter_value.
|
||||
Expr::Is(build_filter(inner))
|
||||
} else {
|
||||
// ref_expr is the only remaining alternative.
|
||||
Expr::Ref(identifier_to_string(first_inner(inner)))
|
||||
@@ -460,6 +463,73 @@ mod tests {
|
||||
assert_eq!(filter.item, "East");
|
||||
}
|
||||
|
||||
// ── IS, the coordinate predicate (improvise-4zl7) ───────────────────
|
||||
|
||||
#[test]
|
||||
fn parse_is_expression() {
|
||||
let f = parse_formula("X = Region IS East", "Cat").unwrap();
|
||||
let Expr::Is(filter) = &f.expr else {
|
||||
panic!("expected an IS, got {:?}", f.expr);
|
||||
};
|
||||
assert_eq!(
|
||||
(filter.category.as_str(), filter.item.as_str()),
|
||||
("Region", "East")
|
||||
);
|
||||
}
|
||||
|
||||
/// The shape the emitter actually writes: a coordinate predicate as the left
|
||||
/// side of an `IF`'s comparison.
|
||||
#[test]
|
||||
fn parse_is_inside_an_if_condition() {
|
||||
let f = parse_formula("X = IF(Region IS East < 0.5, 1, 2)", "Cat").unwrap();
|
||||
let Expr::If(cond, ..) = &f.expr else {
|
||||
panic!("expected an IF, got {:?}", f.expr);
|
||||
};
|
||||
let Expr::BinOp(BinOp::Lt, lhs, _) = &**cond else {
|
||||
panic!("expected a `<`, got {cond:?}");
|
||||
};
|
||||
assert!(matches!(&**lhs, Expr::Is(_)), "got {lhs:?}");
|
||||
}
|
||||
|
||||
/// Both halves quote through `improvise_formula::quote`'s one home, so a name
|
||||
/// with spaces, a pipe or a keyword spelling survives (gotcha 34).
|
||||
#[test]
|
||||
fn parse_is_with_awkward_names() {
|
||||
for (cat, item) in [
|
||||
("Region Name", "East Coast"),
|
||||
("IS", "IF"),
|
||||
("a|b", "c\\d"),
|
||||
("SUM", "9lives"),
|
||||
] {
|
||||
let text = format!(
|
||||
"X = {} IS {}",
|
||||
crate::quote::quote_name(cat),
|
||||
crate::quote::quote_name(item)
|
||||
);
|
||||
let f = parse_formula(&text, "Cat")
|
||||
.unwrap_or_else(|e| panic!("{text:?} did not parse: {e}"));
|
||||
let Expr::Is(filter) = &f.expr else {
|
||||
panic!("{text:?} is not an IS: {:?}", f.expr);
|
||||
};
|
||||
assert_eq!(
|
||||
(filter.category.as_str(), filter.item.as_str()),
|
||||
(cat, item)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// `ref_expr` must not win the race: a bare name still parses as a reference,
|
||||
/// and only the `IS` keyword turns it into a predicate.
|
||||
#[test]
|
||||
fn a_bare_name_is_still_a_reference() {
|
||||
let f = parse_formula("X = Region", "Cat").unwrap();
|
||||
assert!(
|
||||
matches!(f.expr, Expr::Ref(ref n) if n == "Region"),
|
||||
"{:?}",
|
||||
f.expr
|
||||
);
|
||||
}
|
||||
|
||||
// ── Error paths ─────────────────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
|
||||
/// Words the formula grammar gives a meaning of their own. A name spelled like
|
||||
/// one is pipe-quoted even though it is otherwise a legal bare identifier.
|
||||
const KEYWORDS: [&str; 7] = ["WHERE", "SUM", "AVG", "MIN", "MAX", "COUNT", "IF"];
|
||||
const KEYWORDS: [&str; 8] = ["WHERE", "SUM", "AVG", "MIN", "MAX", "COUNT", "IF", "IS"];
|
||||
|
||||
/// Bare-writable: matches `[A-Za-z_][A-Za-z0-9_-]*` and is not a [`KEYWORDS`]
|
||||
/// entry. The keyword clause is canonical — `context/code/formula-and-io.md`.
|
||||
|
||||
@@ -229,6 +229,8 @@ pub fn fit_measure(
|
||||
let header = table.header.clone();
|
||||
let mut data = preprocess(&table.header, &table.rows, &Overrides::default());
|
||||
data.warnings.splice(0..0, table.warnings);
|
||||
no_ambiguous_column_names(&header)?;
|
||||
let features = crate::Features::of(&data, &header[..category_columns]);
|
||||
let rows = regression_rows(data.target, target)?;
|
||||
|
||||
// Non-finite data is a **panic** too, and further down: `standardize` makes NaN
|
||||
@@ -299,12 +301,10 @@ pub fn fit_measure(
|
||||
&mut rng,
|
||||
);
|
||||
|
||||
only_raw_measure_features(&champion, &spec, &header, category_columns, &data.columns)?;
|
||||
|
||||
Ok(FitOutcome {
|
||||
formula: crate::fitted_formula(
|
||||
&champion,
|
||||
&spec,
|
||||
&features,
|
||||
into,
|
||||
improvise_core::model::category::MEASURE_CATEGORY,
|
||||
model,
|
||||
@@ -319,15 +319,9 @@ pub fn fit_measure(
|
||||
})
|
||||
}
|
||||
|
||||
/// Refuse a champion that splits on anything but a **raw numeric measure** column,
|
||||
/// asked of `preprocess`'s plan and by name, never by position. gp.md §11.
|
||||
fn only_raw_measure_features(
|
||||
champion: &gp_tree::Tree,
|
||||
spec: &Spec,
|
||||
header: &[String],
|
||||
category_columns: usize,
|
||||
columns: &[gp_tree::data::ColumnSpec],
|
||||
) -> anyhow::Result<()> {
|
||||
/// Refuse a table that names one column twice. `Features::of` decides category or
|
||||
/// measure by **name**, so an ambiguous name is a spelling with two meanings.
|
||||
fn no_ambiguous_column_names(header: &[String]) -> anyhow::Result<()> {
|
||||
let feature_header = &header[..header.len().saturating_sub(1)];
|
||||
let mut ambiguous: Vec<&str> = feature_header
|
||||
.iter()
|
||||
@@ -346,26 +340,6 @@ fn only_raw_measure_features(
|
||||
"this document names {ambiguous:?} both as a category and as a measure, so a fitted \
|
||||
formula could not say which one it meant. Rename one of them"
|
||||
);
|
||||
let raw_measure_names: Vec<&str> = feature_header
|
||||
.iter()
|
||||
.skip(category_columns)
|
||||
.map(String::as_str)
|
||||
.filter(|name| {
|
||||
columns.iter().any(|c| {
|
||||
c.name == *name && matches!(c.kind, gp_tree::data::ColumnKind::Numeric { .. })
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
let not_measures: Vec<String> = crate::used_features(champion, spec)
|
||||
.into_iter()
|
||||
.filter(|name| !raw_measure_names.contains(&name.as_str()))
|
||||
.collect();
|
||||
anyhow::ensure!(
|
||||
not_measures.is_empty(),
|
||||
"the fitted tree splits on {not_measures:?}, which is not a numeric measure of this \
|
||||
document — a category coordinate or an encoding artefact, and a formula has no way \
|
||||
to spell either. Fit over numeric measures for now"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -517,7 +491,10 @@ mod tests {
|
||||
Expr::If(c, t, e) => [c, t, e].iter().flat_map(|e| leaf_numbers(e)).collect(),
|
||||
Expr::BinOp(_, l, r) => [l, r].iter().flat_map(|e| leaf_numbers(e)).collect(),
|
||||
Expr::UnaryMinus(e) => leaf_numbers(e),
|
||||
Expr::Ref(_) | Expr::Agg(..) => Vec::new(),
|
||||
// The whole tree now sits inside the route-then-aggregate `SUM`, so
|
||||
// stopping at `Agg` would make this walk return nothing at all.
|
||||
Expr::Agg(_, inner, _) => leaf_numbers(inner),
|
||||
Expr::Ref(_) | Expr::Is(_) => Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -646,10 +623,10 @@ mod tests {
|
||||
.expect("distinct numeric values are not distinct categories");
|
||||
}
|
||||
|
||||
/// The other half of the same rule, and the one a *name* cannot catch: a category
|
||||
/// coordinate is refused **by role**, even where an item shares its spelling.
|
||||
/// The spelling of a coordinate and the spelling of a measure cannot collide:
|
||||
/// `Region IS North` is not `|Region=North|` however the document is named.
|
||||
#[test]
|
||||
fn a_category_coordinate_is_refused_even_when_an_item_shares_its_name() {
|
||||
fn a_coordinate_is_spelled_as_one_even_when_an_item_shares_the_one_hots_name() {
|
||||
let (mut model, view) = document();
|
||||
model.category_mut(REGION).expect("Region exists").kind = CategoryKind::Regular;
|
||||
model
|
||||
@@ -680,15 +657,24 @@ mod tests {
|
||||
"the fixture must make the one-hot's name resolvable, or this test \
|
||||
proves nothing the previous one does not"
|
||||
);
|
||||
let err = fit_measure(&model, &view, "Score", FIT_INTO, &request())
|
||||
.expect_err("a one-hot is not a measure however it is spelled");
|
||||
assert!(err.to_string().contains("not a numeric measure"), "{err}");
|
||||
let outcome = fit_measure(&model, &view, "Score", FIT_INTO, &request())
|
||||
.expect("a one-hot over a real category is now spellable");
|
||||
assert!(
|
||||
outcome.formula.raw.contains("Region IS "),
|
||||
"{}",
|
||||
outcome.formula.raw
|
||||
);
|
||||
assert!(
|
||||
!outcome.formula.raw.contains("|Region=North|"),
|
||||
"the coordinate must not be emitted as the measure that shares its name: {}",
|
||||
outcome.formula.raw
|
||||
);
|
||||
}
|
||||
|
||||
/// **The finding that matters most in this whole PR, and a product question
|
||||
/// rather than a defect** (improvise-4zl7). See `context/test-evidence/gp.md`.
|
||||
/// improvise-4zl7's ruling, end to end: the aggregated cell is the sum of the
|
||||
/// per-record predictions. The hand-computed form is in `improvise-core`.
|
||||
#[test]
|
||||
fn a_fitted_rule_at_an_aggregated_cell_routes_on_the_aggregate_not_the_records() {
|
||||
fn a_fitted_rule_at_an_aggregated_cell_routes_per_record_and_sums() {
|
||||
let (mut model, view) = document();
|
||||
let outcome =
|
||||
fit_measure(&model, &view, "Score", FIT_INTO, &request()).expect("the fit lands");
|
||||
@@ -709,20 +695,27 @@ mod tests {
|
||||
.collect();
|
||||
assert_eq!(per_record.len(), 12, "every record must evaluate");
|
||||
|
||||
// One row up — the whole of `Tag` aggregated away — the formula sees
|
||||
// the aggregate of `Cost` over every record and routes once on that.
|
||||
let aggregated = model.evaluate(&CellKey::new(vec![
|
||||
(REGION.to_string(), "North".to_string()),
|
||||
(MEASURE.to_string(), FIT_INTO.to_string()),
|
||||
]));
|
||||
// One row up — the whole of `Tag` aggregated away — the rule is applied
|
||||
// to each record and the answers summed, not applied once to the total.
|
||||
let aggregated = model.evaluate(&CellKey::new(vec![(
|
||||
MEASURE.to_string(),
|
||||
FIT_INTO.to_string(),
|
||||
)]));
|
||||
let sum_of_records: f64 = per_record.iter().sum();
|
||||
// Asserting the negative covers both outcomes and fails the day aggregation
|
||||
// becomes record-wise — which is when this characterization is replaced.
|
||||
assert!(
|
||||
!matches!(aggregated, Some(CellValue::Number(n)) if (n - sum_of_records).abs() <= 1.0),
|
||||
"the aggregated cell now agrees with the per-record predictions \
|
||||
({aggregated:?} vs {sum_of_records}). improvise-4zl7's question \
|
||||
has been answered — replace this characterization with the rule."
|
||||
matches!(aggregated, Some(CellValue::Number(n)) if (n - sum_of_records).abs() <= 1e-6),
|
||||
"the aggregated cell must be the sum of the per-record predictions \
|
||||
({aggregated:?} vs {sum_of_records})"
|
||||
);
|
||||
// And the two really are different code paths: routing once on the total
|
||||
// would land on a single leaf, which cannot be their sum over 12 records.
|
||||
let leaves = leaf_numbers(&outcome.formula.expr);
|
||||
assert!(
|
||||
!leaves
|
||||
.iter()
|
||||
.any(|leaf| (leaf - sum_of_records).abs() <= 1e-6),
|
||||
"the fixture must not make the right answer equal a single leaf, \
|
||||
or this test cannot tell the two rules apart: {leaves:?}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -920,10 +913,10 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-8xhy reaching a real user, and the reason improvise-4zl7 exists.
|
||||
/// What it costs in practice, and the fixture: `context/test-evidence/gp.md`.
|
||||
/// improvise-8xhy reaching a real user, and what improvise-4zl7 bought: the
|
||||
/// fit a category explains now lands. Fixture: `context/test-evidence/gp.md`.
|
||||
#[test]
|
||||
fn a_fit_that_needs_a_category_is_refused_naming_the_feature() {
|
||||
fn a_fit_that_needs_a_category_now_spells_it_as_a_coordinate() {
|
||||
let (mut model, view) = document();
|
||||
// `document()` makes every category a Label; this test is about what
|
||||
// happens when one is not.
|
||||
@@ -949,21 +942,20 @@ mod tests {
|
||||
CellValue::Number(if region == "North" { 1000.0 } else { 4000.0 }),
|
||||
);
|
||||
}
|
||||
let err = fit_measure(&model, &view, "Score", FIT_INTO, &request())
|
||||
.expect_err("the champion can only split on a one-hot");
|
||||
let msg = err.to_string();
|
||||
let outcome = fit_measure(&model, &view, "Score", FIT_INTO, &request())
|
||||
.expect("the champion splits on a one-hot, which is now spellable");
|
||||
let raw = &outcome.formula.raw;
|
||||
assert!(
|
||||
msg.contains("Region=North") || msg.contains("Region=South"),
|
||||
"{msg}"
|
||||
raw.contains("Region IS North") || raw.contains("Region IS South"),
|
||||
"{raw}"
|
||||
);
|
||||
// And it means what it says: the fixture's two groups are 1000 and 4000,
|
||||
// so the two leaves must straddle them rather than sit on one average.
|
||||
let leaves = leaf_numbers(&outcome.formula.expr);
|
||||
assert!(
|
||||
msg.contains("not a numeric measure"),
|
||||
"the refusal must say what kind of thing the feature is: {msg}"
|
||||
leaves.iter().any(|n| *n < 2000.0) && leaves.iter().any(|n| *n > 3000.0),
|
||||
"the rule must separate the two groups, got {leaves:?}"
|
||||
);
|
||||
// The refusal is read off a one-line status bar, so it has to *be* one line
|
||||
// and carry no rustfmt-inserted space runs. See `context/code/gp.md` §14.
|
||||
assert!(!msg.contains('\n'), "{msg:?}");
|
||||
assert!(!msg.contains(" "), "{msg:?}");
|
||||
}
|
||||
|
||||
/// A text-valued target is a *classification* — k trees and an argmax, not
|
||||
|
||||
+252
-94
@@ -7,7 +7,8 @@ pub mod fit;
|
||||
pub use dataset::{FeatureTable, dataset, feature_table};
|
||||
pub use fit::{FitOutcome, FitRequest, Quality, fit_measure, fitted_measure_name};
|
||||
|
||||
use gp_tree::{Spec, Tree};
|
||||
use gp_tree::Tree;
|
||||
use gp_tree::data::{ColumnSpec, Dataset, FeatureRole, MISSING_CAT, feature_roles};
|
||||
use improvise_core::model::Model;
|
||||
use improvise_formula::{Formula, parse_formula};
|
||||
|
||||
@@ -15,59 +16,152 @@ use improvise_formula::{Formula, parse_formula};
|
||||
// that defines it. Re-exported because it was part of this crate's public surface.
|
||||
pub use improvise_formula::quote_name;
|
||||
|
||||
/// The formula text for a fitted tree: `<target> = <nested IFs>`. Preconditions: the
|
||||
/// tree is [`gp_tree::is_valid`] for `spec.features()`, thresholds and leaves finite.
|
||||
pub fn formula_text(tree: &Tree, spec: &Spec, target: &str) -> String {
|
||||
format!("{} = {}", quote_name(target), expr_text(tree, spec))
|
||||
/// How one fitted feature is said in improvise's own vocabulary.
|
||||
/// `context/code/gp.md` §11 carries why each arm asks the document what it asks.
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub enum Spelling {
|
||||
/// `Expr::Ref` — a measure the document must name as an item.
|
||||
Measure(String),
|
||||
/// `Cat IS Item` — a coordinate predicate; the document must have `Cat`.
|
||||
Coordinate { category: String, item: String },
|
||||
/// An encoding artefact: a `col.missing` flag or a date part, which the
|
||||
/// formula language cannot spell at all.
|
||||
Artefact,
|
||||
}
|
||||
|
||||
/// One fitted feature: what `preprocess` called it, and how improvise says it.
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub struct Feature {
|
||||
pub name: String,
|
||||
pub spelling: Spelling,
|
||||
}
|
||||
|
||||
/// The fitted features in spec order — one entry per feature index, so a tree's
|
||||
/// `feat` indexes this directly. Built from the plan, never from the names.
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub struct Features(Vec<Feature>);
|
||||
|
||||
impl Features {
|
||||
/// Read `preprocess`'s own plan. `categories` names the header columns that are
|
||||
/// document *categories*; everything else is a measure column.
|
||||
pub fn of(data: &Dataset, categories: &[String]) -> Self {
|
||||
let features = feature_roles(&data.columns)
|
||||
.into_iter()
|
||||
.zip(&data.feature_names)
|
||||
.map(|(role, name)| Feature {
|
||||
name: name.clone(),
|
||||
spelling: spell(&role, &data.columns, categories),
|
||||
})
|
||||
.collect();
|
||||
Self(features)
|
||||
}
|
||||
|
||||
pub fn len(&self) -> usize {
|
||||
self.0.len()
|
||||
}
|
||||
|
||||
pub fn is_empty(&self) -> bool {
|
||||
self.0.is_empty()
|
||||
}
|
||||
|
||||
fn get(&self, feat: usize) -> &Feature {
|
||||
self.0
|
||||
.get(feat)
|
||||
.expect("tree indexes a feature the spec does not name")
|
||||
}
|
||||
}
|
||||
|
||||
/// A role as improvise says it. A one-hot over a *measure* column names no
|
||||
/// coordinate, and `(missing)` is an absent coordinate — both are artefacts.
|
||||
fn spell(role: &FeatureRole, columns: &[ColumnSpec], categories: &[String]) -> Spelling {
|
||||
let column = &columns[role.column()].name;
|
||||
let is_category = categories.iter().any(|c| c == column);
|
||||
match role {
|
||||
FeatureRole::Numeric { .. } if !is_category => Spelling::Measure(column.clone()),
|
||||
FeatureRole::OneHot { value, .. } if is_category && value != MISSING_CAT => {
|
||||
Spelling::Coordinate {
|
||||
category: column.clone(),
|
||||
item: value.clone(),
|
||||
}
|
||||
}
|
||||
_ => Spelling::Artefact,
|
||||
}
|
||||
}
|
||||
|
||||
/// The formula text for a fitted tree: `<target> = SUM(<nested IFs>)`. The `SUM` is
|
||||
/// the route-then-aggregate rule made syntax — `context/code/core.md` §13.
|
||||
pub fn formula_text(tree: &Tree, features: &Features, target: &str) -> String {
|
||||
format!(
|
||||
"{} = SUM({})",
|
||||
quote_name(target),
|
||||
expr_text(tree, features)
|
||||
)
|
||||
}
|
||||
|
||||
/// [`formula_text`], parsed **and** checked against the document it is to be stored
|
||||
/// in — the form improvise stores. Why, and the interim: `context/code/gp.md` §11.
|
||||
/// in — the form improvise stores. Why, and what is left: `context/code/gp.md` §11.
|
||||
pub fn fitted_formula(
|
||||
tree: &Tree,
|
||||
spec: &Spec,
|
||||
features: &Features,
|
||||
target: &str,
|
||||
target_category: &str,
|
||||
model: &Model,
|
||||
) -> anyhow::Result<Formula> {
|
||||
let unresolvable = unresolvable_features(tree, spec, model);
|
||||
let used = used_features(tree, features);
|
||||
let artefacts: Vec<&str> = used
|
||||
.iter()
|
||||
.filter(|f| f.spelling == Spelling::Artefact)
|
||||
.map(|f| f.name.as_str())
|
||||
.collect();
|
||||
anyhow::ensure!(
|
||||
unresolvable.is_empty(),
|
||||
artefacts.is_empty(),
|
||||
// One line, because it is read off a status bar: a `rustfmt` pass over a
|
||||
// multi-line literal leaves runs of ten spaces mid-sentence.
|
||||
"the fitted tree splits on {artefacts:?}, which the formula language cannot spell \
|
||||
at all — a `col.missing` flag or a date part is an encoding artefact rather than \
|
||||
anything {target:?} could refer to. Fit over measures and categories for now",
|
||||
);
|
||||
let unresolvable: Vec<&str> = used
|
||||
.iter()
|
||||
.filter(|f| !resolves_in(&f.spelling, model))
|
||||
.map(|f| f.name.as_str())
|
||||
.collect();
|
||||
anyhow::ensure!(
|
||||
unresolvable.is_empty(),
|
||||
"the fitted tree splits on {unresolvable:?}, which {} not {} of this document \
|
||||
— a formula refers to its inputs by name, so every cell of {target:?} would be \
|
||||
a reference error. Encoding artefacts (`Cat=Item` one-hots, `col.missing` flags, \
|
||||
date parts) cannot be written as a formula: fit over numeric measures for now",
|
||||
a reference error",
|
||||
if unresolvable.len() == 1 { "is" } else { "are" },
|
||||
if unresolvable.len() == 1 {
|
||||
"an item"
|
||||
"an item or category"
|
||||
} else {
|
||||
"items"
|
||||
"items or categories"
|
||||
},
|
||||
);
|
||||
parse_formula(&formula_text(tree, spec, target), target_category)
|
||||
parse_formula(&formula_text(tree, features, target), target_category)
|
||||
}
|
||||
|
||||
/// Every feature the tree splits on that `Expr::Ref` could not resolve, in the
|
||||
/// order the spec names them and without repeats.
|
||||
fn unresolvable_features(tree: &Tree, spec: &Spec, model: &Model) -> Vec<String> {
|
||||
used_features(tree, spec)
|
||||
.into_iter()
|
||||
.filter(|name| model.find_item_category(name).is_none())
|
||||
.collect()
|
||||
/// Whether the document can answer the spelling. The question is asked **of the
|
||||
/// document**, never of the name's shape — `context/code/gp.md` §11.
|
||||
fn resolves_in(spelling: &Spelling, model: &Model) -> bool {
|
||||
match spelling {
|
||||
Spelling::Measure(name) => model.find_item_category(name).is_some(),
|
||||
Spelling::Coordinate { category, .. } => model.category(category).is_some(),
|
||||
Spelling::Artefact => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Every feature the tree splits on, in spec order and without repeats. **One** walk,
|
||||
/// because two questions are asked of the same set — `context/code/gp.md` §11.
|
||||
pub(crate) fn used_features(tree: &Tree, spec: &Spec) -> Vec<String> {
|
||||
let mut used = vec![false; spec.names.len()];
|
||||
pub(crate) fn used_features<'a>(tree: &Tree, features: &'a Features) -> Vec<&'a Feature> {
|
||||
let mut used = vec![false; features.len()];
|
||||
mark_features(tree, &mut used);
|
||||
spec.names
|
||||
features
|
||||
.0
|
||||
.iter()
|
||||
.zip(&used)
|
||||
.filter(|(_, u)| **u)
|
||||
.map(|(name, _)| (*name).to_string())
|
||||
.map(|(feature, _)| feature)
|
||||
.collect()
|
||||
}
|
||||
|
||||
@@ -83,7 +177,7 @@ fn mark_features(tree: &Tree, used: &mut [bool]) {
|
||||
}
|
||||
}
|
||||
|
||||
fn expr_text(tree: &Tree, spec: &Spec) -> String {
|
||||
fn expr_text(tree: &Tree, features: &Features) -> String {
|
||||
match tree {
|
||||
Tree::Leaf(score) => number_text(*score),
|
||||
Tree::Split {
|
||||
@@ -91,18 +185,26 @@ fn expr_text(tree: &Tree, spec: &Spec) -> String {
|
||||
thresh,
|
||||
lo,
|
||||
hi,
|
||||
} => {
|
||||
let name = spec
|
||||
.names
|
||||
.get(*feat)
|
||||
.expect("tree indexes a feature the spec does not name");
|
||||
format!(
|
||||
"IF({} < {}, {}, {})",
|
||||
quote_name(name),
|
||||
number_text(*thresh),
|
||||
expr_text(lo, spec),
|
||||
expr_text(hi, spec)
|
||||
)
|
||||
} => format!(
|
||||
"IF({} < {}, {}, {})",
|
||||
spelling_text(&features.get(*feat).spelling),
|
||||
number_text(*thresh),
|
||||
expr_text(lo, features),
|
||||
expr_text(hi, features)
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// A spelling as formula text. `Artefact` is a precondition failure here and a
|
||||
/// refusal in [`fitted_formula`] — the same split `number_text` makes.
|
||||
fn spelling_text(spelling: &Spelling) -> String {
|
||||
match spelling {
|
||||
Spelling::Measure(name) => quote_name(name),
|
||||
Spelling::Coordinate { category, item } => {
|
||||
format!("{} IS {}", quote_name(category), quote_name(item))
|
||||
}
|
||||
Spelling::Artefact => {
|
||||
panic!("expr_text wants a feature the formula language can spell")
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -127,7 +229,7 @@ fn number_text(v: f32) -> String {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use gp_tree::fit::{Fit, FitConfig, Row, fit_detailed};
|
||||
use gp_tree::{Rng, SplitMix64, eval, random_tree};
|
||||
use gp_tree::{Rng, Spec, SplitMix64, eval, random_tree};
|
||||
use improvise_core::formula::{Expr, parse_formula};
|
||||
use improvise_core::model::cell::{CellKey, CellValue};
|
||||
use improvise_core::model::types::Model;
|
||||
@@ -139,6 +241,50 @@ mod tests {
|
||||
Spec { ranges, names }
|
||||
}
|
||||
|
||||
/// Features that spell themselves — exactly what `Features::of` builds for a
|
||||
/// numeric measure column, and all most of these tests need.
|
||||
fn measures(names: &[&str]) -> Features {
|
||||
Features(
|
||||
names
|
||||
.iter()
|
||||
.map(|n| Feature {
|
||||
name: (*n).to_string(),
|
||||
spelling: Spelling::Measure((*n).to_string()),
|
||||
})
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
/// One-hot features: what `Features::of` builds for a categorical *category*
|
||||
/// column, named the way `preprocess` names them.
|
||||
fn coordinates(pairs: &[(&str, &str)]) -> Features {
|
||||
Features(
|
||||
pairs
|
||||
.iter()
|
||||
.map(|(cat, item)| Feature {
|
||||
name: format!("{cat}={item}"),
|
||||
spelling: Spelling::Coordinate {
|
||||
category: (*cat).to_string(),
|
||||
item: (*item).to_string(),
|
||||
},
|
||||
})
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Features no formula can spell: a `col.missing` flag or a date part.
|
||||
fn artefacts(names: &[&str]) -> Features {
|
||||
Features(
|
||||
names
|
||||
.iter()
|
||||
.map(|n| Feature {
|
||||
name: (*n).to_string(),
|
||||
spelling: Spelling::Artefact,
|
||||
})
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
fn leaf(v: f32) -> Box<Tree> {
|
||||
Box::new(Tree::Leaf(v))
|
||||
}
|
||||
@@ -183,31 +329,33 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn a_leaf_emits_its_score_and_parses() {
|
||||
let spec = spec_of(&[(0.0, 1.0)], &["x"]);
|
||||
let text = formula_text(&Tree::Leaf(-1.5), &spec, TARGET);
|
||||
assert_eq!(text, "Score = -1.5");
|
||||
let text = formula_text(&Tree::Leaf(-1.5), &measures(&["x"]), TARGET);
|
||||
assert_eq!(text, "Score = SUM(-1.5)");
|
||||
assert!(parse_formula(&text, MEASURE).is_ok(), "{text}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_nested_tree_emits_nested_ifs_and_parses() {
|
||||
let spec = spec_of(&[(0.0, 100.0), (0.0, 1.0)], &["humidity", "windy"]);
|
||||
let features = measures(&["humidity", "windy"]);
|
||||
let tree = split(
|
||||
0,
|
||||
77.5,
|
||||
Box::new(split(1, 0.5, leaf(-0.5), leaf(2.0))),
|
||||
leaf(1.25),
|
||||
);
|
||||
let text = formula_text(&tree, &spec, TARGET);
|
||||
let text = formula_text(&tree, &features, TARGET);
|
||||
assert_eq!(
|
||||
text,
|
||||
"Score = IF(humidity < 77.5, IF(windy < 0.5, -0.5, 2), 1.25)"
|
||||
"Score = SUM(IF(humidity < 77.5, IF(windy < 0.5, -0.5, 2), 1.25))"
|
||||
);
|
||||
let parsed = parse_formula(&text, MEASURE).expect("emitted formula parses");
|
||||
assert_eq!(parsed.target, TARGET);
|
||||
// Nesting survives: the outer IF's `then` branch is itself an IF.
|
||||
let Expr::If(_, then_branch, _) = &parsed.expr else {
|
||||
panic!("expected an IF at the root, got {:?}", parsed.expr);
|
||||
// The route-then-aggregate SUM wraps the tree; nesting survives under it.
|
||||
let Expr::Agg(_, routed, _) = &parsed.expr else {
|
||||
panic!("expected a SUM at the root, got {:?}", parsed.expr);
|
||||
};
|
||||
let Expr::If(_, then_branch, _) = &**routed else {
|
||||
panic!("expected an IF under the SUM, got {routed:?}");
|
||||
};
|
||||
assert!(
|
||||
matches!(**then_branch, Expr::If(..)),
|
||||
@@ -234,15 +382,15 @@ mod tests {
|
||||
"", // empty
|
||||
];
|
||||
for (i, name) in names.iter().enumerate() {
|
||||
let ranges = [(0.0f32, 1.0f32)];
|
||||
let name_slice = [*name];
|
||||
let spec = spec_of(&ranges, &name_slice);
|
||||
let tree = split(0, 0.5, leaf(1.0), leaf(2.0));
|
||||
let text = formula_text(&tree, &spec, TARGET);
|
||||
let text = formula_text(&tree, &measures(&[*name]), TARGET);
|
||||
let parsed = parse_formula(&text, MEASURE)
|
||||
.unwrap_or_else(|e| panic!("name {name:?} (#{i}) emitted {text:?}: {e}"));
|
||||
let Expr::If(cond, ..) = &parsed.expr else {
|
||||
panic!("expected an IF, got {:?}", parsed.expr);
|
||||
let Expr::Agg(_, routed, _) = &parsed.expr else {
|
||||
panic!("expected a SUM, got {:?}", parsed.expr);
|
||||
};
|
||||
let Expr::If(cond, ..) = &**routed else {
|
||||
panic!("expected an IF, got {routed:?}");
|
||||
};
|
||||
let Expr::BinOp(_, lhs, _) = &**cond else {
|
||||
panic!("expected a comparison, got {cond:?}");
|
||||
@@ -258,9 +406,8 @@ mod tests {
|
||||
/// correct. See `context/test-evidence/gp.md`.
|
||||
#[test]
|
||||
fn an_awkward_target_name_is_quoted_and_round_trips() {
|
||||
let spec = spec_of(&[(0.0, 1.0)], &["x"]);
|
||||
let text = formula_text(&Tree::Leaf(1.0), &spec, "Total Revenue");
|
||||
assert_eq!(text, "|Total Revenue| = 1");
|
||||
let text = formula_text(&Tree::Leaf(1.0), &measures(&["x"]), "Total Revenue");
|
||||
assert_eq!(text, "|Total Revenue| = SUM(1)");
|
||||
let parsed = parse_formula(&text, MEASURE).expect("emitted formula parses");
|
||||
assert_eq!(parsed.target, "Total Revenue");
|
||||
}
|
||||
@@ -269,10 +416,15 @@ mod tests {
|
||||
/// `add_formula` puts the target into `_Measure`, where the user can see it.
|
||||
#[test]
|
||||
fn an_awkward_target_names_the_users_measure_item() {
|
||||
let spec = spec_of(&[(0.0, 1.0)], &["x"]);
|
||||
let mut model = Model::new("fit");
|
||||
let formula = fitted_formula(&Tree::Leaf(1.0), &spec, "Total Revenue", MEASURE, &model)
|
||||
.expect("emitted formula parses");
|
||||
let formula = fitted_formula(
|
||||
&Tree::Leaf(1.0),
|
||||
&measures(&["x"]),
|
||||
"Total Revenue",
|
||||
MEASURE,
|
||||
&model,
|
||||
)
|
||||
.expect("emitted formula parses");
|
||||
model.add_formula(formula);
|
||||
assert!(
|
||||
model
|
||||
@@ -317,7 +469,6 @@ mod tests {
|
||||
/// Evidence: `context/test-evidence/gp.md`.
|
||||
#[test]
|
||||
fn a_value_sitting_on_the_threshold_falls_the_same_side() {
|
||||
let spec = spec_of(&[(0.0, 1.0)], &["x"]);
|
||||
let thresh = 0.7f32;
|
||||
// The trap, spelled out: the short rendering is not the same number.
|
||||
assert!(
|
||||
@@ -325,11 +476,11 @@ mod tests {
|
||||
"this test is only meaningful while 0.7 rounds upward"
|
||||
);
|
||||
let tree = split(0, thresh, leaf(-1.0), leaf(1.0));
|
||||
let text = formula_text(&tree, &spec, TARGET);
|
||||
let text = formula_text(&tree, &measures(&["x"]), TARGET);
|
||||
assert_eq!(eval(&tree, &[thresh]), 1.0, "f32 routes an equal value hi");
|
||||
assert_eq!(improvise_eval(&text, &["x"], &[thresh]), 1.0);
|
||||
assert_ne!(
|
||||
text, "Score = IF(x < 0.7, -1, 1)",
|
||||
text, "Score = SUM(IF(x < 0.7, -1, 1))",
|
||||
"0.7 is not the f32's exact value — emitting it is the bug"
|
||||
);
|
||||
}
|
||||
@@ -342,10 +493,11 @@ mod tests {
|
||||
// Deliberately awkward: a space, a keyword, an operator, a one-hot label.
|
||||
const NAMES: [&str; 4] = ["Net Revenue", "MAX", "Cost + Tax", "outlook=rainy"];
|
||||
let spec = spec_of(&RANGES, &NAMES);
|
||||
let features = measures(&NAMES);
|
||||
let mut rng = SplitMix64::new(0x0F17_C0FF_EE00_D00D);
|
||||
for _ in 0..60 {
|
||||
let tree = random_tree(&spec, 5, &mut rng);
|
||||
let text = formula_text(&tree, &spec, TARGET);
|
||||
let text = formula_text(&tree, &features, TARGET);
|
||||
for _ in 0..10 {
|
||||
let f: Vec<f32> = RANGES
|
||||
.iter()
|
||||
@@ -362,14 +514,14 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn fitted_formula_hands_back_a_parsed_formula() {
|
||||
let spec = spec_of(&[(0.0, 1.0)], &["Net Revenue"]);
|
||||
let tree = split(0, 0.5, leaf(1.0), leaf(2.0));
|
||||
let mut model = Model::new("fit");
|
||||
model
|
||||
.category_mut(MEASURE)
|
||||
.expect("_Measure is always present")
|
||||
.add_item("Net Revenue");
|
||||
let formula = fitted_formula(&tree, &spec, TARGET, MEASURE, &model).expect("parses");
|
||||
let formula = fitted_formula(&tree, &measures(&["Net Revenue"]), TARGET, MEASURE, &model)
|
||||
.expect("parses");
|
||||
assert_eq!(formula.target, TARGET);
|
||||
assert_eq!(formula.target_category, MEASURE);
|
||||
}
|
||||
@@ -417,8 +569,9 @@ mod tests {
|
||||
fn the_same_seed_fits_the_same_formula() {
|
||||
let spec = spec_of(&FIT_RANGES, &FIT_NAMES);
|
||||
let rows = fit_rows();
|
||||
let first = formula_text(&fit(&spec, &rows, 0xABCD_1234), &spec, TARGET);
|
||||
let second = formula_text(&fit(&spec, &rows, 0xABCD_1234), &spec, TARGET);
|
||||
let features = measures(&FIT_NAMES);
|
||||
let first = formula_text(&fit(&spec, &rows, 0xABCD_1234), &features, TARGET);
|
||||
let second = formula_text(&fit(&spec, &rows, 0xABCD_1234), &features, TARGET);
|
||||
assert_eq!(first, second);
|
||||
}
|
||||
|
||||
@@ -428,8 +581,9 @@ mod tests {
|
||||
fn a_different_seed_fits_a_different_formula() {
|
||||
let spec = spec_of(&FIT_RANGES, &FIT_NAMES);
|
||||
let rows = fit_rows();
|
||||
let first = formula_text(&fit(&spec, &rows, 0xABCD_1234), &spec, TARGET);
|
||||
let other = formula_text(&fit(&spec, &rows, 0x9999_0001), &spec, TARGET);
|
||||
let features = measures(&FIT_NAMES);
|
||||
let first = formula_text(&fit(&spec, &rows, 0xABCD_1234), &features, TARGET);
|
||||
let other = formula_text(&fit(&spec, &rows, 0x9999_0001), &features, TARGET);
|
||||
assert_ne!(first, other);
|
||||
}
|
||||
|
||||
@@ -452,7 +606,7 @@ mod tests {
|
||||
/// What a fitted formula does inside a user's *document*, as opposed to inside
|
||||
/// `improvise_eval`, which fabricates an item per feature name (improvise-7na).
|
||||
mod evaluability_in_a_real_document {
|
||||
use super::{MEASURE, TARGET, leaf, spec_of, split};
|
||||
use super::{MEASURE, TARGET, artefacts, coordinates, leaf, measures, split};
|
||||
use crate::{fitted_formula, formula_text};
|
||||
use improvise_core::formula::parse_formula;
|
||||
use improvise_core::model::cell::{CellKey, CellValue};
|
||||
@@ -495,11 +649,11 @@ mod tests {
|
||||
/// needs no saved encoding plan. See `context/code/gp.md` §12.
|
||||
#[test]
|
||||
fn a_record_added_after_the_fit_is_scored_by_the_formula_alone() {
|
||||
let spec = spec_of(&[(0.0, 200.0)], &["Cost"]);
|
||||
let tree = split(0, 50.0, leaf(1.0), leaf(2.0));
|
||||
let mut model = document();
|
||||
model.add_formula(
|
||||
parse_formula(&formula_text(&tree, &spec, TARGET), MEASURE).expect("parses"),
|
||||
parse_formula(&formula_text(&tree, &measures(&["Cost"]), TARGET), MEASURE)
|
||||
.expect("parses"),
|
||||
);
|
||||
// A region the fit never saw, added afterwards.
|
||||
model
|
||||
@@ -518,9 +672,8 @@ mod tests {
|
||||
/// fit story works in today.
|
||||
#[test]
|
||||
fn a_split_on_a_measure_evaluates() {
|
||||
let spec = spec_of(&[(0.0, 200.0)], &["Cost"]);
|
||||
let tree = split(0, 50.0, leaf(1.0), leaf(2.0));
|
||||
let text = formula_text(&tree, &spec, TARGET);
|
||||
let text = formula_text(&tree, &measures(&["Cost"]), TARGET);
|
||||
let mut model = document();
|
||||
assert_eq!(
|
||||
score_at(&mut model, &text, "North"),
|
||||
@@ -532,21 +685,29 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-8xhy, the flip of the characterization test this replaces: a
|
||||
/// one-hot name is an encoding artefact no document has an item for.
|
||||
/// improvise-4zl7 flips improvise-8xhy's refusal for this family: a one-hot
|
||||
/// over a real category is a coordinate predicate the grammar now spells.
|
||||
#[test]
|
||||
fn a_split_on_a_one_hot_feature_is_refused() {
|
||||
let spec = spec_of(&[(0.0, 1.0)], &["Region=North"]);
|
||||
fn a_split_on_a_one_hot_feature_is_spelled_as_a_coordinate() {
|
||||
let features = coordinates(&[(REGION, "North")]);
|
||||
let tree = split(0, 0.5, leaf(1.0), leaf(2.0));
|
||||
assert_eq!(
|
||||
formula_text(&tree, &spec, TARGET),
|
||||
"Score = IF(|Region=North| < 0.5, 1, 2)"
|
||||
formula_text(&tree, &features, TARGET),
|
||||
"Score = SUM(IF(Region IS North < 0.5, 1, 2))"
|
||||
);
|
||||
let err = fitted_formula(&tree, &spec, TARGET, MEASURE, &document())
|
||||
.expect_err("a one-hot feature is not an item of the document")
|
||||
assert!(fitted_formula(&tree, &features, TARGET, MEASURE, &document()).is_ok());
|
||||
}
|
||||
|
||||
/// The other half of the same rule: the document must really have the
|
||||
/// category, and the question is asked of the document.
|
||||
#[test]
|
||||
fn a_coordinate_naming_no_category_of_this_document_is_refused() {
|
||||
let features = coordinates(&[("Continent", "Antarctica")]);
|
||||
let tree = split(0, 0.5, leaf(1.0), leaf(2.0));
|
||||
let err = fitted_formula(&tree, &features, TARGET, MEASURE, &document())
|
||||
.expect_err("Continent is not a category of this document")
|
||||
.to_string();
|
||||
assert!(err.contains("Region=North"), "{err}");
|
||||
assert!(err.contains("numeric measures"), "{err}");
|
||||
assert!(err.contains("Continent=Antarctica"), "{err}");
|
||||
}
|
||||
|
||||
/// improvise-hkt6. The same precondition in two places answered differently,
|
||||
@@ -554,9 +715,8 @@ mod tests {
|
||||
#[test]
|
||||
#[should_panic(expected = "tree indexes a feature the spec does not name")]
|
||||
fn a_tree_indexing_past_the_spec_says_so() {
|
||||
let spec = spec_of(&[(0.0, 200.0)], &["Cost"]);
|
||||
let tree = split(7, 50.0, leaf(1.0), leaf(2.0));
|
||||
let _ = fitted_formula(&tree, &spec, TARGET, MEASURE, &document());
|
||||
let _ = fitted_formula(&tree, &measures(&["Cost"]), TARGET, MEASURE, &document());
|
||||
}
|
||||
|
||||
/// The same refusal for the other two artefact families. Nothing here
|
||||
@@ -564,13 +724,12 @@ mod tests {
|
||||
#[test]
|
||||
fn the_missing_flag_and_the_date_sub_features_are_refused_too() {
|
||||
for name in ["Cost.missing", "when.year", "when.month"] {
|
||||
let names = [name];
|
||||
let spec = spec_of(&[(0.0, 1.0)], &names);
|
||||
let tree = split(0, 0.5, leaf(1.0), leaf(2.0));
|
||||
let err = fitted_formula(&tree, &spec, TARGET, MEASURE, &document())
|
||||
let err = fitted_formula(&tree, &artefacts(&[name]), TARGET, MEASURE, &document())
|
||||
.expect_err(name)
|
||||
.to_string();
|
||||
assert!(err.contains(name), "{err}");
|
||||
assert!(err.contains("cannot spell"), "{err}");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -578,10 +737,9 @@ mod tests {
|
||||
/// `Ok`, no cell of the target is a `ref:` error. See the evidence file.
|
||||
#[test]
|
||||
fn a_formula_the_emitter_accepted_evaluates_everywhere() {
|
||||
let spec = spec_of(&[(0.0, 200.0)], &["Cost"]);
|
||||
let tree = split(0, 50.0, leaf(1.0), leaf(2.0));
|
||||
let mut model = document();
|
||||
let formula = fitted_formula(&tree, &spec, TARGET, MEASURE, &model)
|
||||
let formula = fitted_formula(&tree, &measures(&["Cost"]), TARGET, MEASURE, &model)
|
||||
.expect("a split on a real measure is accepted");
|
||||
model.add_formula(formula);
|
||||
model.recompute_formulas(&[]);
|
||||
@@ -597,14 +755,14 @@ mod tests {
|
||||
/// accepted — which is why the check asks the document, not the name's shape.
|
||||
#[test]
|
||||
fn a_measure_that_merely_looks_like_a_one_hot_is_accepted() {
|
||||
let spec = spec_of(&[(0.0, 1.0)], &["Region=North"]);
|
||||
let tree = split(0, 0.5, leaf(1.0), leaf(2.0));
|
||||
let mut model = document();
|
||||
model
|
||||
.category_mut(MEASURE)
|
||||
.expect("_Measure is always present")
|
||||
.add_item("Region=North");
|
||||
assert!(fitted_formula(&tree, &spec, TARGET, MEASURE, &model).is_ok());
|
||||
let features = measures(&["Region=North"]);
|
||||
assert!(fitted_formula(&tree, &features, TARGET, MEASURE, &model).is_ok());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user