From 50ff95fdb0c4d628ff72d44a38d2612a3f4e94ee Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Sat, 29 Aug 2026 02:48:34 -0400 Subject: [PATCH] feat(tactics): load a fitted tactic weighting, and scope what is left `--tactic-weights `, in the same envelope `--weights` uses and read as strictly: an unknown name is refused rather than skipped, because a file the bot shrugged off would have a benchmark measuring the hand-authored set under the fitted one's name. The epic gains the two ways to connect the fitter with what each costs, and the label question written up as jmm's to answer. Change-Id: I56089aa2590fc2bb27086b418de893a94d0842e1 --- crates/sds-bot/src/force.rs | 85 ++++++++++++++++++++- crates/sds-bot/src/main.rs | 64 ++++++++++++++-- crates/sds-core/src/situation.rs | 126 +++++++++++++++++++++++++++++++ plan/tactics.md | 93 ++++++++++++++++++++--- 4 files changed, 353 insertions(+), 15 deletions(-) diff --git a/crates/sds-bot/src/force.rs b/crates/sds-bot/src/force.rs index 04990bc..cd86a43 100644 --- a/crates/sds-bot/src/force.rs +++ b/crates/sds-bot/src/force.rs @@ -111,13 +111,28 @@ impl ForceThinker { } pub fn with_explore(grit: Grit, weights: Weights, explore: Explore) -> Self { + Self::with_tactics(grit, weights, explore, TacticWeights::hand_authored()) + } + + /// The same, with a tactic weighting somebody chose. + /// + /// The one construction path that takes both weight sets. They are separate + /// arguments because they are separate bases - one scores hexes and the + /// other scores tactics - and a single argument holding both would invite a + /// fit against the wrong columns. + pub fn with_tactics( + grit: Grit, + weights: Weights, + explore: Explore, + tactics: TacticWeights, + ) -> Self { Self { commitments: Mutex::new(HashMap::new()), formations: Mutex::new(HashMap::new()), grit, weights, explore, - tactics: TacticWeights::hand_authored(), + tactics, } } } @@ -982,6 +997,74 @@ mod tests { assert_eq!(whole.len(), sds_core::situation::catalogue().len()); } + /// A loaded tactic weighting actually reaches the proposal. + /// + /// The failure this exists to stop is the silent one: a fitted set the bot + /// took and then ignored would have a benchmark measuring the hand-authored + /// weights under the fitted set's name. `load_weights` is loud on every + /// failure for the same reason one layer down. + /// + /// Four scouts are a Recon Lance, which opens on `flank`, and under the + /// hand-authored set that is what the force proposes. Handed a weighting + /// that prices being outnumbered instead, it proposes something else - + /// which is only possible if the weights it was given are the ones it used. + #[test] + fn a_loaded_tactic_weighting_changes_what_a_force_proposes() { + use sds_core::features::Feature as _; + use sds_core::situation::{EnemyShare, IsOurOpening, TacticWeights}; + + let ours: Vec = (1..=4).map(|id| scout(id, "1st Lance")).collect(); + // One enemy against four of ours, so `enemy_share` is low - and a + // weighting with a negative weight on it makes every tactic score the + // same small amount, leaving the fixed order to decide. `engage` is + // first in `orderable`, so that is what comes out. + let theirs = vec![scout(9, "Theirs")]; + + let hand = ForceThinker::default(); + assert_eq!( + hand.consider("1st Lance".into(), 1, &ours, &theirs, None) + .proposal + .tactic, + "flank", + "the baseline no longer proposes the formation's opening" + ); + + // A different set: nothing on the opening, everything on the situation. + let fitted = ForceThinker::with_tactics( + Grit::default(), + Weights::hand_authored(), + sds_core::explore::Explore { + epsilon: 0.0, + seed: 0, + temperature: sds_core::explore::DEFAULT_TEMPERATURE, + }, + TacticWeights::default() + .with::(0.0) + .with::(-1.0), + ); + let proposal = fitted + .consider("1st Lance".into(), 1, &ours, &theirs, None) + .proposal; + assert_eq!( + proposal.tactic, "engage", + "the force ignored the weighting it was given" + ); + // And it is the weighting doing it, not an empty vector: the value is + // the one that set produces rather than nought. + let scored = proposal + .considered + .iter() + .find(|ranked| ranked.tactic == "engage") + .expect("engage was scored"); + assert!( + scored.value < 0.0, + "{} scored {}, which is not what a negative weight gives", + scored.tactic, + scored.value + ); + assert_eq!(IsOurOpening::NAME, "is_our_opening"); + } + /// A force nobody harmonised for falls back to its formation's opening. /// /// The behaviour before there was a coordinator to ask, kept so a node diff --git a/crates/sds-bot/src/main.rs b/crates/sds-bot/src/main.rs index 76f641e..7d3b378 100644 --- a/crates/sds-bot/src/main.rs +++ b/crates/sds-bot/src/main.rs @@ -312,6 +312,41 @@ struct Bot { first_orders_round: i32, } +/// Where the tactic weights come from. +/// +/// The same shape as [`load_weights`] and loud in the same way, so an operator +/// who has learned one weights file has learned both. A fitted tactic set the +/// bot shrugged off would have a benchmark measuring the hand-authored set +/// under the fitted one's name. +fn load_tactic_weights(path: &std::path::Path) -> Result { + let text = std::fs::read_to_string(path) + .with_context(|| format!("cannot read tactic weights file {}", path.display()))?; + let document: serde_json::Value = + serde_json::from_str(&text).with_context(|| format!("{} is not JSON", path.display()))?; + let weights = sds_core::situation::TacticWeights::from_document(&document) + .map_err(|e| anyhow::anyhow!("{}: {e}", path.display()))?; + if weights.is_empty() { + anyhow::bail!("{}: the `weights` object is empty", path.display()); + } + // A set that names fewer features than the basis is legal - a fit may put a + // weight at nought - but it is never intended silently, so it is said with + // the names in it. The same courtesy `load_weights` extends. + let missing: Vec<&str> = sds_core::situation::catalogue() + .into_iter() + .map(|entry| entry.name) + .filter(|name| !weights.names().contains(name)) + .collect(); + if !missing.is_empty() { + eprintln!( + "[sds-bot] tactic weights name {} of {} features; unnamed ones score nought: {}", + weights.names().len(), + sds_core::situation::catalogue().len(), + missing.join(", ") + ); + } + Ok(weights) +} + /// Where the weights come from, and what to say when they cannot be read. /// /// Loud on every failure, on purpose. A weights file the bot shrugged off would @@ -2072,7 +2107,10 @@ fn classify_forces(path: &std::path::Path) -> Result<()> { /// /// Never fatal, and silent when the host did not say where the log is - the /// same rule `write_weights_sidecar` follows. -fn write_tactics_sidecar(arm: sds_core::harmonise::Arm) { +fn write_tactics_sidecar( + arm: sds_core::harmonise::Arm, + weights: &sds_core::situation::TacticWeights, +) { let (Ok(dir), Ok(tag)) = (std::env::var("SDS_LOG_DIR"), std::env::var("SDS_MATCH_TAG")) else { return; }; @@ -2083,7 +2121,7 @@ fn write_tactics_sidecar(arm: sds_core::harmonise::Arm) { "arm": arm.name(), "pinned": arm == sds_core::harmonise::Arm::PinnedEngage, "switchingCost": sds_core::harmonise::SWITCHING_COST, - "weights": sds_core::situation::TacticWeights::hand_authored(), + "weights": weights, "orderable": sds_core::situation::orderable() .iter() .map(|tactic| tactic.name()) @@ -2134,6 +2172,7 @@ async fn run() -> Result<()> { let mut epsilon = 0.0f64; let mut temperature = sds_core::explore::DEFAULT_TEMPERATURE; let mut arm = sds_core::harmonise::Arm::default(); + let mut tactic_weights_path: Option = None; while let Some(arg) = args.next() { match arg.as_str() { "--weights" => { @@ -2191,6 +2230,16 @@ async fn run() -> Result<()> { // `--weights` is one: it is a property of the run somebody chose, // and `sds bench --bot " --tactics engage"` is how an arm // gets recorded in the command the harness stores. + // The fitted counterpart of `--weights`, for the tactic layer's + // own basis. Separate files because they are separate bases: one + // scores hexes and the other scores tactics, and a single file + // holding both would invite a fit against the wrong columns. + "--tactic-weights" => { + let path = args + .next() + .ok_or_else(|| anyhow::anyhow!("--tactic-weights needs a file"))?; + tactic_weights_path = Some(path.into()); + } "--tactics" => { let raw = args .next() @@ -2252,7 +2301,7 @@ async fn run() -> Result<()> { return Ok(()); } other => { - anyhow::bail!("unknown argument {other}; try --weights , --explore or --print-catalogue") + anyhow::bail!("unknown argument {other}; try --weights , --tactic-weights , --tactics , --explore or --print-catalogue") } } } @@ -2263,6 +2312,10 @@ async fn run() -> Result<()> { Weights::hand_authored() } }; + let tactic_weights = match tactic_weights_path { + Some(path) => load_tactic_weights(&path)?, + None => sds_core::situation::TacticWeights::hand_authored(), + }; // The weights, beside the decision log they explain. // @@ -2276,7 +2329,7 @@ async fn run() -> Result<()> { // Written once at startup and never again: it is a few hundred bytes against // a decision log that is megabytes. `sds explain` reads it. write_weights_sidecar(&weights); - write_tactics_sidecar(arm); + write_tactics_sidecar(arm, &tactic_weights); // Said once, on the arm somebody has to be able to read off a match log as // well as out of a file. eprintln!( @@ -2323,10 +2376,11 @@ temperature {temperature}; do not benchmark this run", ); registry.register( "force", - Arc::new(force::ForceThinker::with_explore( + Arc::new(force::ForceThinker::with_tactics( Grit::default(), weights.clone(), explore, + tactic_weights.clone(), )), ); diff --git a/crates/sds-core/src/situation.rs b/crates/sds-core/src/situation.rs index 65149a8..d1a44ac 100644 --- a/crates/sds-core/src/situation.rs +++ b/crates/sds-core/src/situation.rs @@ -568,6 +568,59 @@ impl TacticWeights { .sum() } + /// Every feature this set names. + pub fn names(&self) -> Vec<&str> { + self.by_name.keys().map(String::as_str).collect() + } + + pub fn is_empty(&self) -> bool { + self.by_name.is_empty() + } + + /// Wrap this set the way a weights file on disk is shaped. + /// + /// The same `{"weights": {...}}` envelope `features::Weights` uses, and + /// deliberately: an operator who has learned one weights file has learned + /// both, and `sds explain` reading a directory of them does not need to + /// know which kind it found from the shape. + pub fn to_document(&self) -> serde_json::Value { + serde_json::json!({ "weights": self }) + } + + /// Read one back, refusing anything it cannot account for. + /// + /// Strict for the reason `features::Weights::from_document` is strict: a + /// tactic-weights file the bot shrugged off would make a fit look like it + /// worked and change nothing, and the benchmark would be measuring the + /// hand-authored set under the fitted one's name. + /// + /// A name this build does not have is a hard error rather than a skip. + /// There is no retired-feature list here yet because no feature has ever + /// been retired from this basis; when one is, this is where the same + /// exception goes. + pub fn from_document(document: &serde_json::Value) -> Result { + let object = document + .as_object() + .ok_or(TacticWeightsError::NotAnObject)?; + let map = object + .get("weights") + .and_then(|weights| weights.as_object()) + .ok_or(TacticWeightsError::NoWeightsKey)?; + let known: Vec<&str> = catalogue().into_iter().map(|entry| entry.name).collect(); + let mut weights = TacticWeights::default(); + for (name, value) in map { + let weight = value + .as_f64() + .ok_or_else(|| TacticWeightsError::NotANumber(name.clone()))? + as f32; + if !known.contains(&name.as_str()) { + return Err(TacticWeightsError::UnknownFeature(name.clone())); + } + weights.by_name.insert(name.clone(), weight); + } + Ok(weights) + } + /// The set a force plays with until a fit replaces it. /// /// **Deliberately one number.** A force proposes the tactic its formation @@ -581,6 +634,35 @@ impl TacticWeights { } } +/// What a tactic-weights document can be wrong in. +/// +/// Every feature in this basis is `Bounded` and therefore fittable, so there is +/// no `NotLearnable` arm here - the one `features::WeightsError` needs because +/// its basis has `local` columns in it. If a `local` reading is ever added to +/// the tactic basis, that arm comes with it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum TacticWeightsError { + NotAnObject, + NoWeightsKey, + NotANumber(String), + UnknownFeature(String), +} + +impl std::fmt::Display for TacticWeightsError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + TacticWeightsError::NotAnObject => write!(f, "not a JSON object"), + TacticWeightsError::NoWeightsKey => write!(f, "no `weights` object in it"), + TacticWeightsError::NotANumber(name) => write!(f, "`{name}` is not a number"), + TacticWeightsError::UnknownFeature(name) => { + write!(f, "`{name}` is not a tactic feature this build knows") + } + } + } +} + +impl std::error::Error for TacticWeightsError {} + /// One tactic, scored. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct Ranked { @@ -947,6 +1029,50 @@ mod tests { } } + /// A weights file round-trips through the shape it is stored in. + #[test] + fn a_tactic_weighting_round_trips_through_a_document() { + let weights = TacticWeights::default() + .with::(1.5) + .with::(-0.25); + let document = weights.to_document(); + assert!( + document.get("weights").is_some(), + "the same envelope as Weights" + ); + let read = TacticWeights::from_document(&document).expect("it reads back"); + assert_eq!(read, weights); + assert_eq!(read.get(IsOurOpening::NAME), 1.5); + assert_eq!(read.get(EnemyShare::NAME), -0.25); + } + + /// A name this build does not know is refused, not skipped. + /// + /// The failure this exists to stop is silent: a file the bot shrugged off + /// would have a benchmark measuring the hand-authored set under the fitted + /// one's name. + #[test] + fn a_weights_document_is_read_strictly() { + let bad = serde_json::json!({"weights": {"is_our_opening": 1.0, "los_in": 2.0}}); + assert_eq!( + TacticWeights::from_document(&bad), + Err(TacticWeightsError::UnknownFeature("los_in".into())), + "a task-basis feature was accepted into the tactic basis" + ); + assert_eq!( + TacticWeights::from_document(&serde_json::json!({"weights": {"is_our_opening": "1"}})), + Err(TacticWeightsError::NotANumber("is_our_opening".into())) + ); + assert_eq!( + TacticWeights::from_document(&serde_json::json!({})), + Err(TacticWeightsError::NoWeightsKey) + ); + assert_eq!( + TacticWeights::from_document(&serde_json::json!("weights")), + Err(TacticWeightsError::NotAnObject) + ); + } + /// A tie keeps the fixed order rather than whatever was measured first. #[test] fn a_tie_breaks_on_the_fixed_order() { diff --git a/plan/tactics.md b/plan/tactics.md index 46a91b0..453bbcc 100644 --- a/plan/tactics.md +++ b/plan/tactics.md @@ -430,17 +430,19 @@ fit. Three specific things stand between here and a fit, and none of them is a design problem: -- [ ] **Nothing can load a fitted set.** `ForceThinker` constructs - `TacticWeights::hand_authored()` and there is no flag, no file and no - path that would replace it. `--weights` loads a `features::Weights` and - nothing else. `TacticWeights` derives `Deserialize` already, so this is a - flag and a loader. +- [x] **A fitted set can be loaded.** `--tactic-weights `, in the same + `{"weights": {...}}` envelope `--weights` uses, read strictly - an + unknown name is refused rather than skipped, because a file the bot + shrugged off would have a benchmark measuring the hand-authored set under + the fitted one's name. The run's sidecar records the set that was + actually loaded, so a corpus says which weighting produced it. - [ ] **The fitter cannot see the rows.** `train.py::scan_run` treats a log line as a training row when it has a top-level `candidates` list with a `phi` per entry and a `chosen` index. A force's proposal is nested inside that line's `rationale`, and its keys are `considered`, `features` and a winner named by string rather than by index. The data is recorded; the - reader looks somewhere else. An adapter, not a redesign. + reader looks somewhere else. **Two defensible fixes, scoped below and not + chosen.** - [ ] **There is no label at force granularity.** `label_at` is keyed on seat and round, and a tactic decision is per *force* per round. On a side fielding two lances both would be fitted against one outcome - which is @@ -470,6 +472,78 @@ The obligation seam between forces is there and empty, with the one case that would fill it named. Order-independence across forces is pinned by test and the tests are confirmed to fail against three mutations. +### The reader: two ways, and what each costs + +Not chosen. Both work, they set different conventions, and the choice is worth +making deliberately rather than by whoever writes it first. + +**A. Teach the fitter a second row shape.** `scan_run` gains a branch: a line +with a `rationale` array yields one training row per `ForceDecision` that has a +`proposal`, with `considered` as the candidates, `features` merged with the +hoisted `situation` as the `phi`, and the ordered tactic's position as +`chosen`. + +- *Costs:* a second row shape inside `train.py`, which is the file that has + already been wrong in two expensive ways - the design matrix that met the OOM + reaper, and the two-sided label that fitted seventeen vectors which lost every + decided game. A branch there is a branch in the task layer's fit as well. + `Normals` would also have to be per-basis or run twice, since the two bases + share no columns. +- *Buys:* nothing changes in what the bot writes, so a recording made before + the decision is still usable. + +**B. Emit proposals as first-class rows.** The bot writes one decision-log line +per force per round in the shape `scan_run` already reads - a top-level +`candidates` list, a `phi` per entry, a `chosen` index - and the proposal moves +*out* of `rationale` rather than being copied, so there is one copy and it +cannot drift. + +- *Costs:* a new kind of line in the decision log, which every reader that + assumes a line is a unit decision has to skip. That is the same hazard + `SIDECARS` exists to bound one level up, and it is the kind that fails + silently - a reader counting decisions would count force rows among them. + Moving the proposal also changes what `sds explain` finds where. +- *Buys:* `train.py` needs no change at all to read the rows, and the invariant + "a training row is a line with candidates and a chosen index" stays exactly + one shape. + +**Which I would choose: B**, on the grounds that the invariant is worth more +than the migration - one row shape in the fitter is a property worth keeping, +and `train.py` is the last file in this repository that should grow a branch. +But A is the cheaper change and is right if recordings made before the decision +have to remain fittable, and that is a call about what is already on disk rather +than about the code. + +**Neither should be built before the label question below is settled.** A +credit-assignment scheme that attributes an outcome to a force by, say, its +share of the damage would need that share *in the row*, and a row shape chosen +first would have to be changed again. + +### The label: a design question, and it is jmm's + +`train.py::label_at` is keyed on seat and round. A tactic decision is per +**force** per round. On a side fielding two lances, both would be fitted against +one outcome - one lance's tactic carrying the other lance's luck, which is the +defect `--tier` exists to bound at the unit level, one layer up and unbounded. + +This is not an adapter and it is not a naming mismatch. It is a question about +credit assignment with no obviously right answer, and at least three shapes: + +- **Per seat, as now.** Simplest, and wrong in exactly the measured way: the + same argument that retired the BV differential applies - a label that does not + move with the decision teaches the decision nothing. +- **Attributed by share.** A force's share of the damage its side dealt or took + over the horizon. Needs that share recorded per force per round, which is a + row-shape decision, which is why the reader work waits on this one. +- **Per force, from what that force's own machines did.** The most direct, and + the one that quietly assumes a force's units are the right unit of account - + which is the assumption a `Screen` or a `Bombard` tactic exists to violate, + since both are worth something precisely because of what a *different* force + then manages to do. + +No recommendation here on purpose. Somebody with twenty years of the game and +the whole hierarchy in view should pick it. + ### What it is resting on A fit needs a corpus **recorded with the layer live**, and none exists. It @@ -480,9 +554,10 @@ its own agreement matrix, and it is why the recording had to land before the fit could be argued about. So the honest summary is **not** "the machinery is ready and only the data is -missing". It is: the shape is right, the data is now recorded, and three named -connections - a loader, a reader and a label - are missing before a fit can be -run at all. +missing". It is: the shape is right, the data is now recorded, the loader is +built, and **two connections are missing before a fit can be run at all** - a +reader, scoped above and not chosen, and a label, which is a design question +rather than a piece of work. ## Preconditions -- 2.51.2