From 86e1521f8a258134df62e6bf5894df3cd1cdd9e2 Mon Sep 17 00:00:00 2001 From: Anders Olsson Date: Wed, 9 Sep 2026 21:19:20 +0200 Subject: [PATCH] feat: complete the evidence matrix and add current_skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three of four corners of the evidence matrix existed. The missing one was forward-only *and* key-restricted — which is exactly what per-competitor prequential scoring needs, the intersection of the two workloads `log_evidence_for` and `filtered_log_evidence` are each documented for. `filtered_log_evidence_for` fills it. It is not `log_evidence_internal(true, targets)`: that path selects `skill.forward` as the prior, which stops being a filtering quantity once `iteration` has run a backward sweep. It goes through `filtered_pass` like its unrestricted sibling, with the restriction applied to which events are *scored*, never to which are *run* — so it is a held-out score under the real history, not a score under a counterfactual one where nobody else played. Key resolution for both `*_for` accessors now shares `resolve_targets`, so they cannot drift apart on how an unknown key is reported. `current_skills` is the plural of `current_skill`. Building a leaderboard previously meant materialising every competitor's full smoothed curve via `learning_curves` and reading the last point of each. Tests carry controls in both directions: naming every competitor must recover the unrestricted value (catching a filter that drops too much), and the restricted forward-only value must differ from the restricted smoothed one (catching an alias). Refs #70. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ --- src/history.rs | 98 ++++++++++++++++++++++--- src/time_slice.rs | 22 +++++- tests/evidence_matrix.rs | 151 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 261 insertions(+), 10 deletions(-) create mode 100644 tests/evidence_matrix.rs diff --git a/src/history.rs b/src/history.rs index 6f4cafe..4d71c5f 100644 --- a/src/history.rs +++ b/src/history.rs @@ -1,6 +1,6 @@ use std::{ borrow::Borrow, - collections::{BTreeMap, HashMap}, + collections::{BTreeMap, HashMap, HashSet}, hash::Hash, marker::PhantomData, }; @@ -744,6 +744,35 @@ impl, O: Observer, K: Eq + Hash + Clone> History HashMap { + let mut latest: HashMap = HashMap::new(); + + // Time order, so a later slice overwrites an earlier one. + for slice in &self.time_slices { + for (competitor, skill) in slice.skills.iter() { + latest.insert(competitor, skill.posterior()); + } + } + + latest + .into_iter() + .filter_map(|(competitor, posterior)| { + self.keys.key(competitor).cloned().map(|k| (k, posterior)) + }) + .collect() + } + /// Learning curve for a single key: (time, posterior) pairs in time order. /// /// `None` if the history has never seen the key; `Some(vec![])` if it is @@ -778,7 +807,7 @@ impl, O: Observer, K: Eq + Hash + Clone> History HashMap> { let mut data: HashMap> = HashMap::new(); - for (time, step) in self.filtered_pass() { + for (time, step) in self.filtered_pass(&HashSet::new()) { for (competitor, posterior) in step.posteriors { if let Some(key) = self.keys.key(competitor).cloned() { data.entry(key).or_default().push((time, posterior)); @@ -808,7 +837,7 @@ impl, O: Observer, K: Eq + Hash + Clone> History, O: Observer, K: Eq + Hash + Clone> History, Q: std::hash::Hash + Eq + ?Sized + std::fmt::Debug, { - let mut targets: Vec = Vec::with_capacity(keys.len()); + let targets: Vec = self.resolve_targets(keys)?.into_iter().collect(); + Ok(self.log_evidence_internal(false, &targets)) + } + + /// Intern a key list, refusing the whole list if any key is unknown. + /// + /// Shared by the two `*_for` evidence accessors so they cannot drift apart + /// on how an unknown key is reported. + fn resolve_targets(&self, keys: &[&Q]) -> Result, InferenceError> + where + K: Borrow, + Q: Hash + Eq + ?Sized + std::fmt::Debug, + { + let mut targets = HashSet::with_capacity(keys.len()); for (member, key) in keys.iter().enumerate() { let idx = self .keys @@ -889,22 +931,25 @@ impl, O: Observer, K: Eq + Hash + Clone> History Vec<(T, FilteredStep)> { + /// + /// `targets` restricts each step's evidence sum; the messages carried + /// forward are unaffected. See `TimeSlice::filtered_step`. + fn filtered_pass(&self, targets: &HashSet) -> Vec<(T, FilteredStep)> { let mut messages: HashMap = HashMap::new(); let mut pass = Vec::with_capacity(self.time_slices.len()); for slice in &self.time_slices { - let step = slice.filtered_step(&messages, &self.competitors); + let step = slice.filtered_step(&messages, &self.competitors, targets); for &(competitor, posterior) in &step.posteriors { messages.insert(competitor, posterior); @@ -930,12 +975,47 @@ impl, O: Observer, K: Eq + Hash + Clone> History f64 { - self.filtered_pass() + self.filtered_pass(&HashSet::new()) .iter() .map(|(_, step)| step.log_evidence) .sum() } + /// Filtered log-evidence restricted to events involving at least one of + /// the given keys. + /// + /// The intersection of the two axes the other three evidence accessors + /// span: forward-only *and* key-restricted, which is what per-competitor + /// prequential scoring needs. `log_evidence_for` is the smoothed + /// counterpart, and its per-event priors carry information from events + /// that had not happened yet — so it is not the quantity for scoring a + /// competitor's history *as it unfolded*. + /// + /// Restricting filters which events are *scored*, not which are *run*: the + /// forward messages still absorb every event, so this is a held-out score + /// under the real history, not a score under a counterfactual one where + /// nobody else played. + /// + /// Runs a full forward pass per call and caches nothing. + /// + /// # Errors + /// + /// `UnknownKey` for any key the history has never seen — see + /// `log_evidence_for` for why an unknown key must not be skipped. + pub fn filtered_log_evidence_for(&self, keys: &[&Q]) -> Result + where + K: Borrow, + Q: Hash + Eq + ?Sized + std::fmt::Debug, + { + let targets = self.resolve_targets(keys)?; + + Ok(self + .filtered_pass(&targets) + .iter() + .map(|(_, step)| step.log_evidence) + .sum()) + } + /// The configured observer. /// /// `History` takes its observer by value, so this is how a caller inspects diff --git a/src/time_slice.rs b/src/time_slice.rs index 681eb5d..4ffd6a2 100644 --- a/src/time_slice.rs +++ b/src/time_slice.rs @@ -615,10 +615,18 @@ impl TimeSlice { /// configured prior. The sweep runs on a scratch copy, so the real slice /// is untouched — which is what makes the filtered estimates independent /// of whether `History::converge` has run. + /// One forward-only step for this slice. + /// + /// `targets` restricts only the *evidence sum*, to events in which at + /// least one target competitor appears; an empty set means no restriction. + /// The forward messages are always built from every event in the slice — + /// restricting those instead would answer a different question (a history + /// in which the other events never happened), not a held-out one. pub(crate) fn filtered_step>( &self, incoming: &HashMap, competitors: &CompetitorStore, + targets: &std::collections::HashSet, ) -> FilteredStep { let mut scratch = TimeSlice { events: self.events.clone(), @@ -674,7 +682,19 @@ impl TimeSlice { scratch.iterate_to_convergence(competitors); FilteredStep { - log_evidence: scratch.events.iter().map(|event| event.log_evidence).sum(), + log_evidence: scratch + .events + .iter() + .filter(|event| { + targets.is_empty() + || event + .teams + .iter() + .flat_map(|team| &team.items) + .any(|item| targets.contains(&item.competitor)) + }) + .map(|event| event.log_evidence) + .sum(), posteriors: scratch .skills .iter() diff --git a/tests/evidence_matrix.rs b/tests/evidence_matrix.rs new file mode 100644 index 0000000..249c7df --- /dev/null +++ b/tests/evidence_matrix.rs @@ -0,0 +1,151 @@ +//! The evidence accessors span two independent axes — smoothed vs forward-only, +//! all-keys vs key-restricted — and all four corners must exist and differ. +//! +//! `filtered_log_evidence_for` was the missing corner: the one a per-competitor +//! prequential score needs. + +use trueskill_tt::{ + ConstantDrift, Event, History, InferenceError, Member, NullObserver, Outcome, Team, +}; + +type H = History; + +/// Two disjoint cohorts, so a key restriction is guaranteed to leave events out. +fn two_cohorts() -> H { + let mut h = H::default(); + let mut events = Vec::new(); + for t in 1..=6 { + for (x, y) in [("a", "b"), ("c", "d")] { + events.push(Event { + time: t, + teams: [ + Team::with_members([Member::new(x)]), + Team::with_members([Member::new(y)]), + ] + .into_iter() + .collect(), + outcome: Outcome::winner(0, 2), + }); + } + } + h.add_events(events).expect("fixture ingests"); + h.converge().expect("fixture converges"); + h +} + +#[test] +fn all_four_corners_are_distinct_quantities() { + let h = two_cohorts(); + + let smoothed_all = h.log_evidence(); + let smoothed_ab = h.log_evidence_for(&[&"a", &"b"]).unwrap(); + let filtered_all = h.filtered_log_evidence(); + let filtered_ab = h.filtered_log_evidence_for(&[&"a", &"b"]).unwrap(); + + for (name, v) in [ + ("smoothed_all", smoothed_all), + ("smoothed_ab", smoothed_ab), + ("filtered_all", filtered_all), + ("filtered_ab", filtered_ab), + ] { + assert!( + v.is_finite() && v <= 0.0, + "{name} = {v} is not a log probability" + ); + } + + // Restricting to one cohort must drop the other cohort's events. Half the + // events, and the two cohorts are symmetric, so it lands near half. + assert!( + smoothed_ab > smoothed_all, + "restricting must drop evidence terms: {smoothed_ab} vs {smoothed_all}" + ); + assert!(filtered_ab > filtered_all); + + // The forward-only corner is a genuinely different quantity from the + // smoothed one, not an alias for it. + assert!( + (filtered_ab - smoothed_ab).abs() > 1e-9, + "filtered and smoothed restricted evidence coincide ({filtered_ab} vs {smoothed_ab}); \ + one of them is not computing what it claims" + ); +} + +#[test] +fn restricting_to_both_cohorts_recovers_the_unrestricted_value() { + let h = two_cohorts(); + + // Control on the filter itself: naming every competitor must restrict + // nothing, so this catches a filter that drops events it should keep. + let all_named = h + .filtered_log_evidence_for(&[&"a", &"b", &"c", &"d"]) + .unwrap(); + assert!( + (all_named - h.filtered_log_evidence()).abs() < 1e-12, + "naming everyone changed the answer: {all_named} vs {}", + h.filtered_log_evidence() + ); +} + +/// The restriction selects *events*, not competitors: naming one member of a +/// pair that only ever plays each other selects the same events as naming both. +#[test] +fn naming_either_member_of_a_pair_selects_the_same_events() { + let h = two_cohorts(); + + let ab = h.filtered_log_evidence_for(&[&"a"]).unwrap(); + let ab_pair = h.filtered_log_evidence_for(&[&"a", &"b"]).unwrap(); + assert!( + (ab - ab_pair).abs() < 1e-12, + "a and b only ever play each other, so naming either or both selects \ + the same events: {ab} vs {ab_pair}" + ); +} + +#[test] +fn an_unknown_key_is_an_error_here_too() { + let h = two_cohorts(); + + let err = h + .filtered_log_evidence_for(&[&"typo"]) + .expect_err("unknown key"); + assert!(matches!(err, InferenceError::UnknownKey { .. }), "{err:?}"); + + // Control: the same call on a known key succeeds. + h.filtered_log_evidence_for(&[&"a"]).expect("a is known"); +} + +#[test] +fn current_skills_agrees_with_current_skill() { + let h = two_cohorts(); + + let all = h.current_skills(); + assert_eq!(all.len(), 4, "four competitors played"); + + for key in ["a", "b", "c", "d"] { + let one = h.current_skill(key).expect("played"); + let from_map = all[key]; + assert_eq!( + (one.mu(), one.sigma()), + (from_map.mu(), from_map.sigma()), + "current_skills disagrees with current_skill for {key}" + ); + } +} + +#[test] +fn current_skills_omits_a_registered_but_unplayed_competitor() { + let mut h = two_cohorts(); + h.register(Member::new("e")).expect("e is new"); + + let all = h.current_skills(); + assert!( + !all.contains_key("e"), + "a competitor with no appearances has no posterior to report" + ); + assert!( + h.current_skill("e").is_none(), + "control: the singular agrees" + ); + assert_eq!(all.len(), 4); +}