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