From 9c39d1e681ce61398ad61a33ead2ac31549f8e38 Mon Sep 17 00:00:00 2001 From: Anders Olsson Date: Thu, 27 Aug 2026 16:37:21 +0200 Subject: [PATCH] test: pin the invariants that make filtered estimates trustworthy The bracket test proves the feature works on one fixture. These pin the bug class: - Invariance to converge(). This is the one that matters. Reading skill.forward instead of the carried message makes it fail immediately, because converge() alternates sweeps and contaminates skill.forward with backward information from the second iteration onward. That is the property a stored field cannot have, and the reason issue #19's proposed fix would not have worked. - Invariance to ingestion order, the crate's standing invariant. - One slice has no future to propagate back, so filtered equals smoothed. - Empty history yields zero and empty maps. Agreement is to 1e-8 under tight convergence rather than bit-identity: iteration recomputes the colour partition only when from == 0, so an incrementally built slice keeps insertion order until the first converge() reorders it, and the scratch clone inherits whichever order it finds. Same fixed point, different path to it. --- tests/degenerate_inputs.rs | 11 +++ tests/filtered.rs | 151 +++++++++++++++++++++++++++++++++++-- 2 files changed, 155 insertions(+), 7 deletions(-) diff --git a/tests/degenerate_inputs.rs b/tests/degenerate_inputs.rs index bd666a6..1c4fa14 100644 --- a/tests/degenerate_inputs.rs +++ b/tests/degenerate_inputs.rs @@ -245,3 +245,14 @@ fn log_evidence_finite_for_near_certain_outcome() { upset.log_evidence() ); } + +#[test] +fn empty_history_has_no_filtered_estimates() { + let history: History = History::builder().build(); + + assert_eq!(history.filtered_log_evidence(), 0.0); + + assert!(history.filtered_learning_curves().is_empty()); + + assert!(history.filtered_learning_curve("nobody").is_empty()); +} diff --git a/tests/filtered.rs b/tests/filtered.rs index ca5ff22..452f0a0 100644 --- a/tests/filtered.rs +++ b/tests/filtered.rs @@ -2,14 +2,12 @@ //! as opposed to the smoothed posteriors `learning_curve` reports. use smallvec::smallvec; -use trueskill_tt::{Event, History, Member, Outcome, Team}; +use trueskill_tt::{ConvergenceOptions, Event, History, Member, Outcome, Team}; -/// `games` one-on-one matches at successive times, won by "a" every time. -/// -/// This is the fixture from issue #19, where `online(true)` reported -/// `games * ln(0.5)`. -fn repeated_winner(games: i64) -> History { - let mut history = History::builder().build(); +/// `games` one-on-one matches at successive times, won by "a" every time, +/// built with the given convergence options. +fn repeated_winner_with(games: i64, convergence: ConvergenceOptions) -> History { + let mut history = History::builder().convergence(convergence).build(); for time in 1..=games { history @@ -27,6 +25,24 @@ fn repeated_winner(games: i64) -> History { history } +/// `games` one-on-one matches at successive times, won by "a" every time. +/// +/// This is the fixture from issue #19, where `online(true)` reported +/// `games * ln(0.5)`. +fn repeated_winner(games: i64) -> History { + repeated_winner_with(games, ConvergenceOptions::default()) +} + +/// The default 30-iteration cap leaves a residual around 1e-6, which would +/// swamp these comparisons. Drive both sides well past the fixed point. +fn tight() -> ConvergenceOptions { + ConvergenceOptions { + max_iter: 2_000, + epsilon: 1e-12, + ..ConvergenceOptions::default() + } +} + #[test] fn filtered_evidence_sits_between_coin_flip_and_batch() { let mut history = repeated_winner(5); @@ -115,3 +131,124 @@ fn filtered_curves_plural_agrees_with_singular() { "the plural form must agree with the singular for the same key" ); } + +#[test] +fn filtered_evidence_is_invariant_to_convergence() { + let mut history = repeated_winner_with(6, tight()); + + let before = history.filtered_log_evidence(); + + let report = history.converge().unwrap(); + assert!( + report.converged, + "fixture must converge: {:?}", + report.final_step + ); + + let after = history.filtered_log_evidence(); + + assert!( + (before - after).abs() < 1e-8, + "filtered evidence moved across converge(): {before} -> {after}. The pass must \ + carry its own forward messages; anything reading skill.forward shows exactly \ + this drift, because converge() contaminates it with backward information." + ); +} + +#[test] +fn single_slice_filtered_matches_smoothed() { + let mut history = History::builder().convergence(tight()).build(); + + history + .add_events([ + Event { + time: 1, + teams: smallvec![ + Team::with_members([Member::new("a")]), + Team::with_members([Member::new("b")]), + ], + outcome: Outcome::winner(0, 2), + }, + Event { + time: 1, + teams: smallvec![ + Team::with_members([Member::new("c")]), + Team::with_members([Member::new("d")]), + ], + outcome: Outcome::winner(0, 2), + }, + ]) + .unwrap(); + + history.converge().unwrap(); + + let smoothed = history.learning_curve("a"); + let filtered = history.filtered_learning_curve("a"); + + assert_eq!(smoothed.len(), 1); + assert_eq!(filtered.len(), 1); + + assert!( + (smoothed[0].1.mu() - filtered[0].1.mu()).abs() < 1e-8 + && (smoothed[0].1.sigma() - filtered[0].1.sigma()).abs() < 1e-8, + "one slice has no future to propagate back, so filtered and smoothed must \ + agree: smoothed mu={} sigma={}, filtered mu={} sigma={}", + smoothed[0].1.mu(), + smoothed[0].1.sigma(), + filtered[0].1.mu(), + filtered[0].1.sigma() + ); +} + +#[test] +fn filtered_curves_do_not_depend_on_ingestion_order() { + let events = |time: i64, winner: &'static str, loser: &'static str| Event { + time, + teams: smallvec![ + Team::with_members([Member::new(winner)]), + Team::with_members([Member::new(loser)]), + ], + outcome: Outcome::winner(0, 2), + }; + + let all = vec![ + events(1, "a", "b"), + events(1, "c", "d"), + events(1, "a", "c"), + events(1, "b", "d"), + events(2, "a", "d"), + events(2, "b", "c"), + events(2, "a", "b"), + ]; + + let mut batched = History::builder().convergence(tight()).build(); + batched.add_events(all.clone()).unwrap(); + batched.converge().unwrap(); + + let mut incremental = History::builder().convergence(tight()).build(); + for event in all { + incremental.add_events([event]).unwrap(); + } + incremental.converge().unwrap(); + + let from_batched = batched.filtered_learning_curve("a"); + let from_incremental = incremental.filtered_learning_curve("a"); + + assert_eq!(from_batched.len(), from_incremental.len()); + + for ((time_b, gaussian_b), (time_i, gaussian_i)) in + from_batched.iter().zip(from_incremental.iter()) + { + assert_eq!(time_b, time_i); + + assert!( + (gaussian_b.mu() - gaussian_i.mu()).abs() < 1e-8 + && (gaussian_b.sigma() - gaussian_i.sigma()).abs() < 1e-8, + "at t={time_b}: batched mu={} sigma={}, incremental mu={} sigma={}", + gaussian_b.mu(), + gaussian_b.sigma(), + gaussian_i.mu(), + gaussian_i.sigma() + ); + } +}