Merge pull request #25 from fiddlerwoaroof/wave/m-gp-tree

WU-M: wire gp-tree into improvise (improvise-szrt, improvise-vx3; improvise-21d's last open child).

improvise-4zl7 stays open and grew: a formula evaluated at an aggregated cell routes on the aggregate, so aggregate-then-route is not route-then-aggregate and EVERY fitted tree — numeric splits included — means one thing on a record cell and another one row up. The bead asked this for categorical splits; it is a question about the feature.
This commit is contained in:
Ed Langley
2026-09-19 23:52:22 -07:00
committed by GitHub
10 changed files with 2168 additions and 32 deletions
Generated
+1
View File
@@ -862,6 +862,7 @@ dependencies = [
"anyhow",
"improvise-core",
"improvise-formula",
"improvise-gp",
"improvise-io",
"improvise-protocol",
"serde",
+11 -27
View File
@@ -83,8 +83,8 @@ use gp_tree::data::{
read_csv,
};
use gp_tree::fit::{
FitConfig, Row, argmax, cv_classify, cv_regress, fit, misclass_rate, one_vs_rest,
permutation_importance, standardize,
FitConfig, RegressionQuality, Row, argmax, cv_classify, cv_regress, fit, importance_ranking,
misclass_rate, one_vs_rest, permutation_importance, regression_quality, standardize,
};
use gp_tree::{Spec, SplitMix64, Tree, eval, is_valid, render, size};
@@ -137,16 +137,8 @@ fn confidence(scores: &[f32]) -> f32 {
/// Print the most important features (by permutation importance), as a share of
/// total importance with a little bar. Zero-importance features are omitted.
fn report_importance(names: &[String], imp: &[f32]) {
let total: f32 = imp.iter().sum::<f32>().max(1e-9);
let mut order: Vec<usize> = (0..imp.len()).collect();
order.sort_by(|&a, &b| {
imp[b]
.partial_cmp(&imp[a])
.unwrap_or(std::cmp::Ordering::Equal)
});
println!("feature importance (permutation):");
for &j in order.iter().take(12).filter(|&&j| imp[j] > 0.0) {
let share = 100.0 * imp[j] / total;
for (j, share) in importance_ranking(imp).into_iter().take(12) {
println!(
" {:<24} {share:>5.1}% {}",
names[j],
@@ -157,22 +149,14 @@ fn report_importance(names: &[String], imp: &[f32]) {
/// Print pooled-CV regression metrics against the predict-the-mean baseline.
fn report_regression_cv(pooled: &[(f32, f32)]) {
let n = pooled.len() as f32;
let ybar = pooled.iter().map(|&(_, a)| a).sum::<f32>() / n;
let ss_res: f32 = pooled.iter().map(|&(p, a)| (p - a).powi(2)).sum();
let ss_tot: f32 = pooled.iter().map(|&(_, a)| (a - ybar).powi(2)).sum();
let mse = ss_res / n;
let baseline = ss_tot / n; // MSE of always predicting the mean
let r2 = if ss_tot > 0.0 {
1.0 - ss_res / ss_tot
} else {
0.0
};
println!(
" held-out MSE {mse:.4} RMSE {:.4} R\u{00b2} {r2:.3}",
mse.sqrt()
);
println!(" baseline (predict mean) MSE {baseline:.4}");
let RegressionQuality {
mse,
rmse,
r2,
baseline_mse,
} = regression_quality(pooled);
println!(" held-out MSE {mse:.4} RMSE {rmse:.4} R\u{00b2} {r2:.3}");
println!(" baseline (predict mean) MSE {baseline_mse:.4}");
}
/// Print pooled-CV classification metrics: accuracy vs the majority-class
+257
View File
@@ -254,6 +254,114 @@ pub fn standardize(rows: &[Row<f32>]) -> (Vec<Row<f32>>, f32, f32) {
(normalized, mean, std)
}
/// The inverse of [`standardize`], applied to a tree rather than to rows.
///
/// A fit runs against a standardized target, so its leaf scores are in
/// standard deviations and a caller reading them as the user's units is out by
/// an affine map. `fit_csv` closes that by *printing* the map beside the tree;
/// a caller that has to hand the tree to something else — improvise writes it
/// into a document as a formula — has nowhere to print it, so the map has to be
/// folded into the leaves instead.
///
/// Splits are untouched, and that is the whole reason this is sound:
/// [`standardize`] rescales only the target, so a threshold is already in the
/// feature's own units and re-scaling it would be the bug.
pub fn denormalize(t: &Tree, mean: f32, std: f32) -> Tree {
match t {
Tree::Leaf(s) => Tree::Leaf(s * std + mean),
Tree::Split {
feat,
thresh,
lo,
hi,
} => Tree::Split {
feat: *feat,
thresh: *thresh,
lo: Box::new(denormalize(lo, mean, std)),
hi: Box::new(denormalize(hi, mean, std)),
},
}
}
/// How well a set of pooled `(prediction, actual)` pairs did, **and what doing
/// nothing would have scored**.
///
/// The baseline is the point. A tree with a plausible-looking RMSE that is no
/// better than always predicting the mean is not a weak rule, it is *no rule* —
/// and it is worse than no rule at all, because it is one a user will believe.
/// So the comparison is part of the answer rather than something a caller may
/// forget to compute.
#[derive(Clone, Copy, Debug, PartialEq)]
pub struct RegressionQuality {
/// Held-out mean squared error.
pub mse: f32,
/// Its square root, in the target's own units.
pub rmse: f32,
/// `1 - ss_res/ss_tot`: the share of the target's variance explained.
/// Zero when the target does not vary, and **negative** when the fit does
/// worse than the mean — which is a real outcome, not a clamped one.
pub r2: f32,
/// The mean squared error of always predicting the target's mean.
pub baseline_mse: f32,
}
/// [`RegressionQuality`] over pooled cross-validation pairs.
///
/// Empty pairs give all-zero, via [`mean_over`]: `cv_regress` pools nothing
/// below two folds, and "no evidence" reads the same as "explained nothing".
pub fn regression_quality(pooled: &[(f32, f32)]) -> RegressionQuality {
let n = pooled.len();
let ybar = mean_over(pooled.iter().map(|&(_, a)| a).sum::<f32>(), n);
let ss_res: f32 = pooled.iter().map(|&(p, a)| (p - a).powi(2)).sum();
let ss_tot: f32 = pooled.iter().map(|&(_, a)| (a - ybar).powi(2)).sum();
let mse = mean_over(ss_res, n);
RegressionQuality {
mse,
rmse: mse.sqrt(),
r2: if ss_tot > 0.0 {
1.0 - ss_res / ss_tot
} else {
0.0
},
baseline_mse: mean_over(ss_tot, n),
}
}
/// [`permutation_importance`] scores turned into a ranking: `(feature index,
/// percentage share)`, most important first, with the features that moved the
/// error not at all dropped.
///
/// The share is of the total importance, so it reads as "this feature carried
/// 40% of what the model knows". A total of zero — every feature irrelevant, or
/// a tree that is one leaf — gives an empty ranking rather than a division by
/// zero.
///
/// That zero case is answered by **returning early**, not by flooring the
/// divisor. `fit_csv` used `.max(1e-9)`, which is right for the only input it
/// ever had and wrong as a contract: a lone importance of `1e-10` then divides
/// by the floor rather than by itself and reports **10%**, where the one
/// feature the model uses carries all of it (Copilot on PR #25). Importances
/// this small are ordinary — they are error differences over a standardized
/// target — so the shares have to sum to 100% at every scale, not only at the
/// CLI's.
pub fn importance_ranking(imp: &[f32]) -> Vec<(usize, f32)> {
let total: f32 = imp.iter().sum();
if total <= 0.0 {
return Vec::new();
}
let mut order: Vec<usize> = (0..imp.len()).collect();
order.sort_by(|&a, &b| {
imp[b]
.partial_cmp(&imp[a])
.unwrap_or(std::cmp::Ordering::Equal)
});
order
.into_iter()
.filter(|&j| imp[j] > 0.0)
.map(|j| (j, 100.0 * imp[j] / total))
.collect()
}
/// Split `0..n` into `k` folds after a deterministic shuffle, so held-out rows
/// are a representative sample rather than a contiguous block.
///
@@ -418,6 +526,155 @@ pub fn permutation_importance<T: Clone>(
imp
}
#[cfg(test)]
mod quality_tests {
use super::*;
/// The baseline is the whole point of the type. A "prediction" that is
/// always the target's mean explains nothing, and the numbers have to say
/// so: R² exactly zero, and the fit's MSE equal to the baseline's.
#[test]
fn predicting_the_mean_scores_zero_and_ties_the_baseline() {
let actual = [1.0f32, 2.0, 3.0, 4.0];
let mean = 2.5;
let q = regression_quality(&actual.map(|a| (mean, a)));
assert_eq!(q.r2, 0.0);
assert_eq!(q.mse, q.baseline_mse);
assert_eq!(q.rmse, q.mse.sqrt());
}
/// A perfect fit is R² 1 and zero error, while the baseline still carries
/// the target's own variance — so the two really are measuring different
/// things rather than both reading off the predictions.
#[test]
fn a_perfect_fit_is_one_and_the_baseline_keeps_the_variance() {
let q = regression_quality(&[(1.0, 1.0), (2.0, 2.0), (3.0, 3.0), (4.0, 4.0)]);
assert_eq!(q.r2, 1.0);
assert_eq!(q.mse, 0.0);
assert!(q.baseline_mse > 1.0, "{q:?}");
}
/// Worse than the mean is a real outcome and is reported as one. Clamping
/// it at zero would make "learned nothing" and "actively misleading" read
/// alike, which is the failure this whole report exists to prevent.
#[test]
fn a_fit_worse_than_the_mean_reports_a_negative_r2() {
let q = regression_quality(&[(4.0, 1.0), (1.0, 4.0)]);
assert!(q.r2 < 0.0, "{q:?}");
assert!(q.mse > q.baseline_mse, "{q:?}");
}
/// A target that never varies has no variance to explain, so R² is defined
/// as zero rather than as a division by zero.
#[test]
fn a_constant_target_explains_nothing_rather_than_dividing_by_zero() {
let q = regression_quality(&[(7.0, 7.0), (7.0, 7.0)]);
assert_eq!(q.r2, 0.0);
assert_eq!(q.baseline_mse, 0.0);
}
/// `cv_regress` pools nothing below two folds, so the caller can be handed
/// an empty slice. All-zero, and no NaN — the `mean_over` rule.
#[test]
fn no_pairs_is_all_zero_and_never_nan() {
let q = regression_quality(&[]);
assert_eq!(q.mse, 0.0);
assert_eq!(q.rmse, 0.0);
assert_eq!(q.r2, 0.0);
assert_eq!(q.baseline_mse, 0.0);
}
/// Shares are of the total, most important first, and a feature the model
/// does not use at all is left out rather than listed at 0%.
#[test]
fn the_ranking_is_ordered_shares_with_the_unused_features_dropped() {
let ranked = importance_ranking(&[1.0, 0.0, 3.0]);
assert_eq!(ranked.len(), 2, "the zero-importance feature is listed");
assert_eq!(ranked[0].0, 2);
assert_eq!(ranked[1].0, 0);
assert!((ranked[0].1 - 75.0).abs() < 1e-3, "{ranked:?}");
assert!((ranked[1].1 - 25.0).abs() < 1e-3, "{ranked:?}");
}
/// Every feature irrelevant — or a champion that is a single leaf, which
/// scores every shuffle identically — is an empty ranking, not a division
/// by zero.
#[test]
fn nothing_important_ranks_nothing() {
assert!(importance_ranking(&[0.0, 0.0]).is_empty());
}
/// Copilot on PR #25. The shares must sum to 100% at **every** scale, and
/// `fit_csv`'s `.max(1e-9)` floor made that true only above the floor: one
/// feature carrying an importance of `1e-10` carries all of what the model
/// knows, and used to be reported at 10%. Importances this small are
/// ordinary — they are error differences over a *standardized* target.
#[test]
fn a_tiny_lone_importance_is_still_the_whole_of_it() {
let ranked = importance_ranking(&[1e-10]);
assert_eq!(ranked.len(), 1);
assert!((ranked[0].1 - 100.0).abs() < 1e-3, "{ranked:?}");
}
/// The general form of the same claim, across four orders of magnitude.
#[test]
fn the_shares_sum_to_a_hundred_at_any_scale() {
for scale in [1e-12f32, 1e-6, 1.0, 1e6] {
let total: f32 = importance_ranking(&[scale, 3.0 * scale])
.iter()
.map(|&(_, share)| share)
.sum();
assert!((total - 100.0).abs() < 1e-2, "at {scale}: {total}");
}
}
/// `denormalize` is the inverse of the affine map `standardize` applied,
/// so a tree fitted on standardized targets predicts in the user's own
/// units — which is the only form that can be written into a document.
/// The *splits* must not move: `standardize` rescales the target only, so
/// a threshold is already in its feature's units.
#[test]
fn denormalize_rescales_the_leaves_and_leaves_the_splits_alone() {
let t = Tree::Split {
feat: 0,
thresh: 0.5,
lo: Box::new(Tree::Leaf(-1.0)),
hi: Box::new(Tree::Leaf(2.0)),
};
let back = denormalize(&t, 100.0, 10.0);
assert_eq!(eval(&back, &[0.0]), 90.0);
assert_eq!(eval(&back, &[1.0]), 120.0);
let Tree::Split { thresh, .. } = back else {
panic!("denormalize turned a split into a leaf");
};
assert_eq!(thresh, 0.5, "the threshold was rescaled with the leaves");
}
/// The round trip that earns the function: standardize a large-magnitude
/// target, fit *something* over it, denormalize, and the tree predicts the
/// original units. Done against a hand-built tree rather than a search, so
/// it pins the arithmetic and not the GA.
#[test]
fn a_tree_fitted_on_standardized_rows_predicts_original_units_after_denormalize() {
let rows: Vec<Row<f32>> = [1000.0f32, 2000.0, 3000.0, 4000.0]
.iter()
.enumerate()
.map(|(i, &t)| Row::new(vec![i as f32], t))
.collect();
let (normalized, mean, std) = standardize(&rows);
// The exact rule, expressed in standardized space.
let standardized_tree = Tree::Split {
feat: 0,
thresh: 1.5,
lo: Box::new(Tree::Leaf(normalized[0].target)),
hi: Box::new(Tree::Leaf(normalized[3].target)),
};
let real = denormalize(&standardized_tree, mean, std);
assert!((eval(&real, &[0.0]) - 1000.0).abs() < 0.5);
assert!((eval(&real, &[3.0]) - 4000.0).abs() < 0.5);
}
}
#[cfg(test)]
mod tests {
use super::*;
+1
View File
@@ -11,6 +11,7 @@ improvise-core = { path = "../improvise-core" }
improvise-protocol = { path = "../improvise-protocol" }
improvise-formula = { path = "../improvise-formula" }
improvise-io = { path = "../improvise-io" }
improvise-gp = { path = "../improvise-gp" }
anyhow = "1"
serde = { version = "1", features = ["derive"] }
serde_json = "1"
@@ -0,0 +1,383 @@
//! `fit-measure`: evolve a decision tree over what the active view shows and
//! store it as an ordinary improvise formula.
//!
//! After this command there is nothing GP-specific left in the document — the
//! rule is a `## Formulas` line like any other, recomputed by the fixed-point
//! evaluator, editable in the formula panel, drillable, and saved with no new
//! grammar (improvise-21d's enabling fact, and improvise-7na's answer).
//!
//! # The fit runs inside `execute`, and that is the architecture rather than a
//! shortcut
//!
//! A `Cmd` is pure over `&CmdContext` and returns effects; an `Effect` is the
//! only mutation site. A fit is **decision logic** — a pure function of
//! `(Model, View, target, seed, FitConfig)` that mutates nothing — so it
//! belongs on the command side, and what the effects carry is its *result*.
//! Putting the search inside an `Effect::apply` would have mixed an expensive
//! stochastic computation into the one place the app mutates, which is the
//! split's whole point.
//!
//! What this does **not** buy is asynchrony. The command runs on the thread
//! that draws the next frame, so a fit is bounded rather than cancellable:
//! [`FitRequest::default`] is a deliberately smaller search than the CLI's
//! (improvise-vx3's AC 4 asks for "cancellable **or** bounded"). Moving it off
//! the main loop is a change to `draw.rs`'s event loop, not to this file, and
//! nothing here forecloses it — the fit is already a pure function that could
//! be handed to a worker and have its `FitOutcome` posted back as the same
//! effects.
//!
//! # What a refusal is for
//!
//! `improvise_gp::fit_measure` fails rather than storing something misleading
//! — most sharply when the champion splits on an encoding artefact like
//! `Region=North`, which is not an item of anyone's document, so every cell of
//! the stored measure would be `Error("ref:…")` (improvise-8xhy, repo-map
//! gotcha 37d). This command is where a user actually meets that restriction,
//! so the refusal goes to the status line **whole**: it names the features that
//! could not be resolved, because "the fit failed" is not something a user can
//! act on and "it split on Region, which is a category rather than a measure"
//! is.
//!
//! # What registering it costs the browser, measured
//!
//! `improvise-command` is in the **worker** bundle, and the path from
//! `App::new` → `default_keymaps` → `default_registry` → this command's
//! prototype is live, so `gp-tree` links into it whether or not a browser ever
//! invokes `fit-measure`. `kb.pl` names keeping GP out of the wasm bundle as
//! `improvise-gp`'s reason to exist, so this is a cost to state rather than to
//! discover: built with `wasm-pack --release`, `improvise_worker_server_bg.wasm`
//! goes **1,341,130 → 1,420,470 bytes** (+79 KB, +5.9%), or **498 → 531 KB
//! gzipped** (+6.5%). `improvise-wasm-client` — the main-thread bundle — does
//! not depend on `improvise-command` at all and is **unchanged**.
//!
//! Accepted rather than feature-gated, for two reasons. The worker already
//! carries the whole `App`, persistence and the import wizard, so this is a
//! 6% addition to something an order of magnitude larger; and a browser fit is
//! a *wanted* end state, not dead weight — `improvise_gp::fit` exists where it
//! does precisely so a second host can run the same sequence. If the bundle
//! ever needs shrinking, a default-on cargo feature that `web/build.sh` turns
//! off is the lever, and this paragraph is the number to beat.
use improvise_gp::fit::one_line;
use improvise_gp::{FitRequest, fit_measure, fitted_measure_name};
use crate::command::cmd::core::{Cmd, CmdContext};
use crate::ui::effect::{self, Effect};
/// A seed as the status line spells it (`0x…`, either case) or in decimal.
///
/// The hex arm is not a convenience: `FitOutcome::status_line` prints `{:#x}`
/// and tells the user that is what reproduces the fit, so accepting only
/// decimal made the advertised round trip fail on the advertised value.
fn parse_seed(text: &str) -> Result<u64, String> {
let parsed = match text.strip_prefix("0x").or_else(|| text.strip_prefix("0X")) {
Some(hex) => u64::from_str_radix(hex, 16),
None => text.parse::<u64>(),
};
parsed.map_err(|e| format!("fit-measure: {text:?} is not a seed: {e}"))
}
/// Fit `target` from the other measures and categories the view shows, storing
/// the rule as the measure `into`.
#[derive(Debug)]
pub struct FitMeasure {
pub target: String,
pub into: String,
pub request: FitRequest,
}
impl FitMeasure {
/// A nameless instance for `register_pure`, which reads a command's name
/// off a prototype rather than off a string literal (gotcha 15).
pub fn prototype() -> Self {
Self {
target: String::new(),
into: String::new(),
request: FitRequest::default(),
}
}
/// `fit-measure <target> [<measure-name>] [<seed>]`.
///
/// The seed is an argument rather than a clock read for the reason the CLI
/// fixes its own default: two fits of the same document should agree, and a
/// user who wants a different search asks for one. It is reported back on
/// the status line so the fit that landed can be asked for again — **which
/// is why it is read in the form it is printed**, hexadecimal with a `0x`
/// prefix, as well as decimal. `status_line` spells it `{:#x}`, so a
/// decimal-only parser made the one instruction the status line gives a
/// user fail on the value the status line just showed them.
pub fn parse(args: &[String]) -> Result<Box<dyn Cmd>, String> {
let [target, rest @ ..] = args else {
return Err("fit-measure requires a target measure".into());
};
let (into, seed) = match rest {
[] => (fitted_measure_name(target), None),
[into] => (into.clone(), None),
[into, seed] => (into.clone(), Some(parse_seed(seed)?)),
_ => {
return Err(format!(
"fit-measure takes 1-3 arguments, got {}",
args.len()
));
}
};
Ok(Box::new(FitMeasure {
target: target.clone(),
into,
request: FitRequest {
seed: seed.unwrap_or(FitRequest::default().seed),
..FitRequest::default()
},
}))
}
}
impl Cmd for FitMeasure {
fn name(&self) -> &'static str {
"fit-measure"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
match fit_measure(ctx.model, ctx.view, &self.target, &self.into, &self.request) {
// The report first and `AddFormula` last, so a formula the emitter
// spelled but the parser will not take leaves *its* message on
// screen rather than this command's optimistic one (gotcha 20).
Ok(outcome) => vec![
effect::set_status(outcome.status_line()),
Box::new(effect::MarkDirty),
Box::new(effect::AddFormula {
raw: outcome.formula.raw.clone(),
target_category: outcome.formula.target_category.clone(),
}),
],
// Flattened like the success line: several refusals interpolate a
// document name, and a name may legally contain a newline
// (Copilot on PR #25). A failure is when a user is most reading
// the status bar, so it is the last place to let one split.
Err(e) => vec![effect::set_status(one_line(format!("fit-measure: {e}")))],
}
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::command::cmd::test_helpers::*;
use crate::model::category::CategoryKind;
use crate::model::cell::{CellKey, CellValue};
use crate::view::Axis;
const MEASURE: &str = "_Measure";
const REGION: &str = "Region";
const TAG: &str = "Tag";
/// A document a tree can learn exactly — `Score` steps at `Cost` 50 — in
/// the units a real document uses.
///
/// **Every category is a `Label`**, so the only feature is the numeric
/// measure `Cost` and the champion cannot reach an unresolvable one. An
/// earlier version left `Region` regular with one item and claimed
/// `simplify` would prune the constant one-hot; it does not, because
/// `build_categorical` records `(0.0, 1.0)` for every one-hot whatever the
/// column holds — so these tests were passing on seed luck (Copilot on
/// PR #25). See `improvise_gp::fit`'s own fixture for the same correction.
fn workbook() -> crate::workbook::Workbook {
let mut wb = crate::workbook::Workbook::new("sales");
wb.add_category(REGION).expect("first category");
wb.add_item(REGION, "North").expect("Region exists");
wb.add_category(TAG).expect("second category");
for cat in [REGION, TAG] {
wb.model.category_mut(cat).expect("just added").kind = CategoryKind::Label;
}
for i in 0..12u32 {
wb.add_item(TAG, format!("r{i}")).expect("Tag exists");
}
let measure = wb
.model
.category_mut(MEASURE)
.expect("_Measure always exists");
for item in ["Cost", "Score"] {
measure.add_item(item);
}
for i in 0..12u32 {
let cost = 10.0 * f64::from(i);
wb.model.set_cell(cell(i, "Cost"), CellValue::Number(cost));
wb.model.set_cell(
cell(i, "Score"),
CellValue::Number(if cost > 50.0 { 4000.0 } else { 1000.0 }),
);
}
wb.active_view_mut().set_axis(REGION, Axis::Row);
wb
}
fn cell(i: u32, measure: &str) -> CellKey {
CellKey::new(vec![
(REGION.to_string(), "North".to_string()),
(TAG.to_string(), format!("r{i}")),
(MEASURE.to_string(), measure.to_string()),
])
}
fn cmd(args: &[&str]) -> Box<dyn Cmd> {
FitMeasure::parse(&args.iter().map(|s| s.to_string()).collect::<Vec<_>>())
.expect("the arguments parse")
}
/// Run `fit-measure` the way the app does: build the read-only context
/// from a workbook, execute, and apply the effects to a real `App`.
fn run(args: &[&str]) -> crate::ui::app::App {
let wb = workbook();
let layout = make_layout(&wb);
let registry = make_registry();
let effects = cmd(args).execute(&make_ctx(&wb, &layout, &registry));
let mut app = crate::ui::app::App::new(workbook(), None);
app.apply_effects(effects);
app
}
/// The end-to-end claim of improvise-vx3, asserted against a real
/// document: a command fits, the formula lands through the ordinary
/// `add_formula` path, and the new measure evaluates to numbers.
#[test]
fn the_command_installs_a_formula_the_document_evaluates() {
let app = run(&["Score"]);
let model = &app.model_state.workbook.model;
assert!(
model.formulas().iter().any(|f| f.target == "Score (fit)"),
"no fitted formula was stored: {:?} / {:?}",
model.formulas().iter().map(|f| &f.raw).collect::<Vec<_>>(),
app.view_state.status_msg,
);
for i in 0..12u32 {
match model.evaluate(&cell(i, "Score (fit)")) {
Some(CellValue::Number(_)) => {}
other => panic!("record r{i} came back as {other:?}"),
}
}
}
/// Adding a rule is a change to the document, so the command marks it
/// dirty — otherwise the fit is lost on the next quit without a prompt.
#[test]
fn a_fit_marks_the_document_dirty() {
assert!(
!crate::ui::app::App::new(workbook(), None).model_state.dirty,
"the fixture starts clean"
);
assert!(run(&["Score"]).model_state.dirty);
}
/// improvise-vx3 is emphatic that the rule alone is not the deliverable:
/// a tidy tree that generalizes no better than the mean is worse than
/// useless. So the status line carries the held-out comparison, what the
/// rule leans on, and the seed that reproduces it.
#[test]
fn the_status_line_reports_quality_against_the_baseline() {
let app = run(&["Score"]);
let status = &app.view_state.status_msg;
for expected in ["R\u{b2}", "baseline", "leans on", "seed"] {
assert!(
status.contains(expected),
"{expected:?} missing from {status:?}"
);
}
}
/// A refusal reaches the user *whole*. "fit failed" is not actionable;
/// naming the measure that does not exist is (improvise-8xhy's argument,
/// arriving at the place a user meets it).
#[test]
fn a_refusal_names_what_went_wrong_and_stores_nothing() {
let wb = workbook();
let layout = make_layout(&wb);
let registry = make_registry();
let effects = cmd(&["Margin"]).execute(&make_ctx(&wb, &layout, &registry));
assert_eq!(effects.len(), 1, "a refused fit must emit no model change");
let mut app = crate::ui::app::App::new(workbook(), None);
app.apply_effects(effects);
assert!(
app.view_state
.status_msg
.contains("not a measure holding data"),
"{:?}",
app.view_state.status_msg
);
assert!(app.model_state.workbook.model.formulas().is_empty());
}
/// gotcha 37b: the seed is half of what reproduces a fit, so it is an
/// argument — and a different one really does search differently.
///
/// It asserts on the **rule**, never on the batch's debug string. The
/// first version compared `effects_debug` over the whole vec, which was
/// vacuous: `status_line` prints `seed {:#x}`, so the two strings differ
/// because the seed was *printed*, and `fit_measure` could have ignored
/// `request.seed` entirely with the test still green
/// (`doctrine(check_the_checker)`).
///
/// `AddFormula` has exactly two fields — the formula text and the target
/// category — and the seed reaches neither, so its `Debug` cannot differ
/// for the reason the batch's did. (A substring guard was tried and is
/// wrong: the seeds `1` and `99` occur inside perfectly ordinary
/// thresholds.)
#[test]
fn the_seed_argument_changes_the_search() {
let wb = workbook();
let layout = make_layout(&wb);
let registry = make_registry();
let ctx = make_ctx(&wb, &layout, &registry);
let rule = |seed: &str| {
cmd(&["Score", "A", seed])
.execute(&ctx)
.iter()
.map(|e| format!("{e:?}"))
.find(|d| d.starts_with("AddFormula"))
.expect("a successful fit adds a formula")
};
assert_ne!(
rule("1"),
rule("99"),
"two seeds produced the identical rule"
);
}
#[test]
fn a_target_is_required_and_a_bad_seed_is_named() {
assert!(FitMeasure::parse(&[]).is_err());
let err = FitMeasure::parse(&["Score".into(), "A".into(), "later".into()])
.expect_err("\"later\" is not a seed");
assert!(err.contains("is not a seed"), "{err}");
}
/// Copilot on PR #25. The status line advertises the seed as `{:#x}` and
/// tells the user that is what reproduces the fit, so the one instruction
/// it gives has to work on the value it just showed them — a decimal-only
/// parser made copying it back an error.
///
/// Asserted as a *round trip through the status line*, not against a
/// hand-written `"0x…"`: a literal would still pass if `status_line`
/// changed how it spells the number, which is the half that breaks.
#[test]
fn the_seed_the_status_line_prints_is_a_seed_this_command_accepts() {
let app = run(&["Score"]);
let printed = app
.view_state
.status_msg
.split_whitespace()
.last()
.expect("the status line ends with the seed")
.to_string();
assert!(printed.starts_with("0x"), "{printed:?}");
assert_eq!(
parse_seed(&printed),
Ok(FitRequest::default().seed),
"the status line's own seed did not parse back"
);
// …and the ordinary decimal form still works, in both directions.
assert_eq!(parse_seed("99"), Ok(99));
assert_eq!(parse_seed("0X1f"), Ok(31));
assert!(parse_seed("0xnope").is_err());
}
}
@@ -2,6 +2,7 @@ pub mod cell;
pub mod commit;
pub mod core;
pub mod effect_cmds;
pub mod fit;
pub mod grid;
pub mod mode;
pub mod navigation;
@@ -15,6 +15,7 @@ use super::cell::*;
use super::commit::*;
use super::core::*;
use super::effect_cmds::*;
use super::fit::FitMeasure;
use super::grid::*;
use super::mode::*;
use super::navigation::*;
@@ -54,6 +55,7 @@ pub fn default_registry() -> CmdRegistry {
},
);
r.register_pure(&AddFormulaCmd(vec![]), AddFormulaCmd::parse);
r.register_pure(&FitMeasure::prototype(), FitMeasure::parse);
r.register_pure(&RemoveFormulaCmd(vec![]), RemoveFormulaCmd::parse);
r.register_pure(&CreateViewCmd(vec![]), CreateViewCmd::parse);
r.register_pure(&DeleteViewCmd(vec![]), DeleteViewCmd::parse);
+61 -3
View File
@@ -113,12 +113,23 @@ fn cell_text(value: &CellValue) -> String {
}
}
/// The coordinates every visible record shares, exactly as `GridLayout::new`
/// derives them: one selection per filtering category, defaulting to its first
/// item when the view names none.
/// The coordinates every visible record shares — one selection per filtering
/// category, defaulting to its first item when the view names none, exactly as
/// `GridLayout::new` derives them, **except** that `_Measure` never
/// constrains.
///
/// That exception is not a special case, it is the difference between a grid
/// and a feature matrix. `_Measure` is the coordinate the *columns* are spread
/// across, so pinning it to one item is not "which records am I looking at" but
/// "throw away every column except this one" — and `Workbook::new`'s default
/// view puts it on `Axis::Page`, so that was the behaviour on a freshly opened
/// document. Either the target's own column survived and every feature came
/// back blank, or, on a document whose target is not `_Measure`'s first item,
/// the table was empty and the error blamed the data.
fn filter_coords(model: &Model, view: &View) -> Vec<(String, String)> {
view.filtering_cats()
.into_iter()
.filter(|cat| !CategoryKind::of_name(cat).is_measure())
.filter_map(|cat| {
let selected = view
.page_selection(cat)
@@ -395,6 +406,53 @@ mod tests {
);
}
/// Regression (improvise-vx3): **`_Measure` is a filtering category and
/// must not filter a fit.** `Workbook::new`'s default view puts it on
/// `Axis::Page`, so `filtering_cats()` named it, `filter_coords` pinned it
/// to the first measure, and every "record" then carried exactly one
/// measure cell — never the target. The whole table came back as "no
/// visible record has a Revenue value to learn from" on a document plainly
/// full of them, which reads as a data problem rather than as an axis one.
///
/// The rule that fixes it is not a special case: a fit's target *and* its
/// features are measures, spread across the `_Measure` coordinate, so
/// paging to one is not "which records am I looking at" but "throw away
/// every column". Page ∪ Filter still scopes the rows in every other
/// category, which is what the test below this one pins.
///
/// Every test in this module used `View::new` + `on_category_added`, under
/// which a leading-underscore category lands on `Axis::None` — so the
/// fixture was quietly building the one world the code worked in, and no
/// test here could see the default a real `Workbook` hands out.
#[test]
fn a_measure_on_the_page_axis_does_not_filter_away_the_training_rows() {
let (model, mut view) = fixture();
let unfiltered = table(&model, &view);
view.set_axis(MEASURE_CATEGORY, Axis::Page);
let paged = feature_table(&model, &view, "Revenue")
.expect("paging the measure axis is not a reason to have no data");
// The *contents*, not the shape. Pinning only the header and the row
// count passes on this fixture even with the bug present: `_Measure`'s
// first item is `Revenue`, so every row still has a target and only
// the feature columns come back blank — the shape survives while the
// data does not, which is exactly the silent failure to guard against.
assert_eq!(paged, unfiltered);
}
/// The other half, and the sharper one: which measure `filter_coords`
/// would have pinned to is the *first* item, so on a document whose target
/// is not first the table came back empty and the error blamed the data —
/// "no visible record has a Revenue value to learn from", on a document
/// plainly full of them.
#[test]
fn a_paged_measure_that_is_not_the_target_does_not_empty_the_table() {
let (model, mut view) = fixture();
let unfiltered = table(&model, &view);
view.set_axis(MEASURE_CATEGORY, Axis::Page);
view.set_page_selection(MEASURE_CATEGORY, "Cost");
assert_eq!(table(&model, &view), unfiltered);
}
/// gotcha 27: what is *visible* is Page ∪ Filter. Scoping by
/// `categories_on(Axis::Page)` would fit against rows a drill has
/// already excluded, and say nothing about it.
File diff suppressed because it is too large Load Diff
+25 -2
View File
@@ -84,8 +84,10 @@
//! it looks and what it would and would not cover.
pub mod dataset;
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 improvise_core::model::Model;
@@ -153,7 +155,13 @@ pub fn fitted_formula(
let unresolvable = unresolvable_features(tree, spec, model);
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",
// One line, because it is read off a status bar. A `rustfmt` pass over
// the previous multi-line literal left runs of ten spaces mid-sentence
// in the message a user actually sees.
"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",
if unresolvable.len() == 1 { "is" } else { "are" },
if unresolvable.len() == 1 {
"an item"
@@ -167,12 +175,27 @@ pub fn fitted_formula(
/// 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()
}
/// Every feature the tree splits on, in the order the spec names them and
/// without repeats.
///
/// Public to the crate because two different questions are asked of the same
/// set: whether the *document* can resolve each name (here) and whether
/// `preprocess`'s own plan says each one is a raw numeric measure
/// ([`fit::fit_measure`]). Walking the tree twice, two ways, is how those two
/// answers would drift apart.
pub(crate) fn used_features(tree: &Tree, spec: &Spec) -> Vec<String> {
let mut used = vec![false; spec.names.len()];
mark_features(tree, &mut used);
spec.names
.iter()
.zip(&used)
.filter(|(name, u)| **u && model.find_item_category(name).is_none())
.filter(|(_, u)| **u)
.map(|(name, _)| (*name).to_string())
.collect()
}