From 13a395fdc91502f2e1c807500eca35bbbc4f4dd1 Mon Sep 17 00:00:00 2001 From: Anders Olsson Date: Wed, 9 Sep 2026 23:23:12 +0200 Subject: [PATCH] refactor!: scores_with_noise, and History::quality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two names that described the wrong thing. `scores_with_sigma(scores, sigma)` reads as "these scores have prior sigma 2.0". The quantity is observation noise on the score *margin*, in the units of the scores, and it is spelled `score_sigma` at every config site — `HistoryBuilder::score_sigma`, `GameOptions::score_sigma`, `EventKind::Scored { score_sigma }` — so this was the one place the crate used a third meaning of "sigma" for it. Its own doc had to disambiguate itself: "`sigma` overrides `HistoryBuilder::score_sigma`". `scores_with_noise(scores, score_sigma)` on both `Outcome` and `EventBuilder`. `predict_quality` predicts nothing. Its own doc says it answers "is this matchup *fair*", not "what will happen", and the `predict_*` family is otherwise exactly the methods returning a probability or a distribution over outcomes. `History::quality` also makes the free/method pair consistent: free `quality` pairs with `History::quality` the way free `expected_information_gain` already pairs with `History::expected_information_gain`. The rule that was already being followed and never stated — a free function scores a hypothetical from explicit parameters, the same-named method asks it against the fit — is now written on the method. Closes #75. Refs #78 (part 4). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ --- src/event_builder.rs | 22 +++++++++++++++------- src/history.rs | 33 ++++++++++++++++++++------------- src/outcome.rs | 13 +++++++++---- tests/api_shape.rs | 2 +- tests/constructor_validation.rs | 4 ++-- tests/degenerate_inputs.rs | 2 +- tests/key_ergonomics.rs | 2 +- tests/prediction.rs | 4 ++-- tests/prediction_guards.rs | 4 ++-- tests/quality.rs | 7 ++----- tests/validation.rs | 4 ++-- 11 files changed, 57 insertions(+), 40 deletions(-) diff --git a/src/event_builder.rs b/src/event_builder.rs index dddc2de..e866dc6 100644 --- a/src/event_builder.rs +++ b/src/event_builder.rs @@ -171,13 +171,21 @@ where /// Set explicit per-team continuous scores with a per-event noise override. /// - /// `sigma` overrides `HistoryBuilder::score_sigma` for this event only. - /// Must be `> 0.0`. Constructing the outcome with a non-positive or NaN - /// sigma is allowed; the value is rejected with - /// `InferenceError::InvalidParameter` when the event is ingested, so - /// callers get an error from `commit` rather than a panic. - pub fn scores_with_sigma>(mut self, scores: I, sigma: f64) -> Self { - self.event.outcome = crate::Outcome::scores_with_sigma(scores, sigma); + /// `score_sigma` is the observation noise on the *score margin*, not a + /// skill sigma, and it overrides `HistoryBuilder::score_sigma` for this + /// event only. A small value takes the margin near-literally; a large one + /// barely moves the ratings. + /// + /// Must be `> 0.0`. Building the outcome with a non-positive or NaN value + /// is allowed; it is rejected with `InferenceError::InvalidParameter` when + /// the event is ingested, so callers get an error from `commit` rather + /// than a panic. + pub fn scores_with_noise>( + mut self, + scores: I, + score_sigma: f64, + ) -> Self { + self.event.outcome = crate::Outcome::scores_with_noise(scores, score_sigma); self } diff --git a/src/history.rs b/src/history.rs index 3b88cf3..0f334ef 100644 --- a/src/history.rs +++ b/src/history.rs @@ -1188,7 +1188,7 @@ impl, O: Observer, K: Eq + Hash + Clone> History, O: Observer, K: Eq + Hash + Clone> History, O: Observer, K: Eq + Hash + Clone> History, O: Observer, K: Eq + Hash + Clone> History(&self, teams: &[&[&Q]]) -> Result + pub fn quality(&self, teams: &[&[&Q]]) -> Result where K: Borrow, Q: Hash + Eq + ?Sized + std::fmt::Debug, @@ -1826,7 +1833,7 @@ impl, O: Observer, K: Eq + Hash + Clone> History 0.0`. Constructing an `Outcome` with a /// non-positive or NaN value is allowed; the value is rejected with /// `InferenceError::InvalidParameter` when the event is ingested, so /// callers get an error rather than a panic. - pub fn scores_with_sigma>(scores: I, score_sigma: f64) -> Self { + pub fn scores_with_noise>(scores: I, score_sigma: f64) -> Self { Self::Scored { scores: scores.into_iter().collect(), score_sigma: Some(score_sigma), @@ -211,7 +216,7 @@ mod tests { #[test] fn scores_with_sigma_round_trips() { - let o = Outcome::scores_with_sigma([10.0, 4.0], 0.5); + let o = Outcome::scores_with_noise([10.0, 4.0], 0.5); assert_eq!(o.team_count(), 2); assert_eq!(o.as_scores(), Some(&[10.0, 4.0][..])); } @@ -227,7 +232,7 @@ mod tests { #[test] fn scores_with_sigma_sets_sigma_some() { - let o = Outcome::scores_with_sigma([3.0, 1.0], 2.0); + let o = Outcome::scores_with_noise([3.0, 1.0], 2.0); match o { Outcome::Scored { score_sigma, .. } => assert_eq!(score_sigma, Some(2.0)), Outcome::Ranked(_) => panic!("expected Scored variant"), @@ -239,7 +244,7 @@ mod tests { /// `tests/degenerate_inputs.rs::scored_event_rejects_non_positive_sigma`. #[test] fn scores_with_sigma_defers_validation_to_ingestion() { - let o = Outcome::scores_with_sigma([3.0, 1.0], 0.0); + let o = Outcome::scores_with_noise([3.0, 1.0], 0.0); match o { Outcome::Scored { score_sigma, .. } => assert_eq!(score_sigma, Some(0.0)), Outcome::Ranked(_) => panic!("expected Scored variant"), diff --git a/tests/api_shape.rs b/tests/api_shape.rs index 5e9fb01..3ec6f57 100644 --- a/tests/api_shape.rs +++ b/tests/api_shape.rs @@ -203,7 +203,7 @@ fn predict_quality_two_teams() { h.record_winner(&"a", &"b", 1).unwrap(); let _ = h.converge().unwrap(); - let q = h.predict_quality(&[&[&"a"], &[&"b"]]).unwrap(); + let q = h.quality(&[&[&"a"], &[&"b"]]).unwrap(); assert!(q > 0.0 && q <= 1.0); } diff --git a/tests/constructor_validation.rs b/tests/constructor_validation.rs index 9a0574e..3c41b52 100644 --- a/tests/constructor_validation.rs +++ b/tests/constructor_validation.rs @@ -102,7 +102,7 @@ fn every_magnitude_parameter_rejects_a_negative_value() { }), ), ( - "Outcome::scores_with_sigma (at ingestion)", + "Outcome::scores_with_noise (at ingestion)", Box::new(|v| { let mut h = History::builder().build(); h.add_events(vec![trueskill_tt::Event { @@ -111,7 +111,7 @@ fn every_magnitude_parameter_rejects_a_negative_value() { trueskill_tt::Team::with_members([Member::new("a")]), trueskill_tt::Team::with_members([Member::new("b")]), ], - outcome: Outcome::scores_with_sigma([3.0, 1.0], v), + outcome: Outcome::scores_with_noise([3.0, 1.0], v), }]) .is_err() }), diff --git a/tests/degenerate_inputs.rs b/tests/degenerate_inputs.rs index ec1acaf..9adf95b 100644 --- a/tests/degenerate_inputs.rs +++ b/tests/degenerate_inputs.rs @@ -222,7 +222,7 @@ fn scored_event_rejects_non_positive_sigma() { .event(1) .team(["a"]) .team(["b"]) - .scores_with_sigma([3.0, 1.0], f64::NAN) + .scores_with_noise([3.0, 1.0], f64::NAN) .commit() .unwrap_err(); assert!(matches!( diff --git a/tests/key_ergonomics.rs b/tests/key_ergonomics.rs index 2baea2f..5faaa08 100644 --- a/tests/key_ergonomics.rs +++ b/tests/key_ergonomics.rs @@ -52,7 +52,7 @@ fn every_team_shaped_query_accepts_the_same_slice() { let h = owned(); let teams: &[&[&str]] = &[&["alice"], &["bob"]]; - h.predict_quality(teams).expect("quality"); + h.quality(teams).expect("quality"); let _ = h.predict_outcome(teams).expect("outcome"); h.predict_ranking(teams, &[0, 1]).expect("ranking"); h.expected_information_gain(teams) diff --git a/tests/prediction.rs b/tests/prediction.rs index c18e5d1..dd41b2f 100644 --- a/tests/prediction.rs +++ b/tests/prediction.rs @@ -34,7 +34,7 @@ fn unknown_keys_are_reported_not_silently_dropped() { h.predict_win_probabilities(&[&[&"a"], &[&"ghost"]]) .is_err() ); - assert!(h.predict_quality(&[&[&"a"], &[&"ghost"]]).is_err()); + assert!(h.quality(&[&[&"a"], &[&"ghost"]]).is_err()); assert!(h.predict_ranking(&[&[&"a"], &[&"ghost"]], &[0, 1]).is_err()); } @@ -407,7 +407,7 @@ fn prior_reaches_every_prediction_entry_point() { let h = history_with_policy(&["a", "b"], trueskill_tt::UnknownKeys::Prior); let teams: &[&[&&str]] = &[&[&"a"], &[&"ghost"]]; - assert!(h.predict_quality(teams).is_ok()); + assert!(h.quality(teams).is_ok()); assert!(h.predict_win_probabilities(teams).is_ok()); assert!(h.predict_outcome(teams).is_ok()); assert!(h.predict_ranking(teams, &[0, 1]).is_ok()); diff --git a/tests/prediction_guards.rs b/tests/prediction_guards.rs index 7eaff7e..189cf60 100644 --- a/tests/prediction_guards.rs +++ b/tests/prediction_guards.rs @@ -78,7 +78,7 @@ macro_rules! all_predictions { ($h:ident, $f:expr) => {{ let teams: &[&[&&'static str]] = &[&[&"a"], &[&"b"]]; let f = $f; - f("predict_quality", $h.predict_quality(teams).map(|_| ())); + f("quality", $h.quality(teams).map(|_| ())); f( "predict_win_probabilities", $h.predict_win_probabilities(teams).map(|_| ()), @@ -116,7 +116,7 @@ fn degenerate_performances_are_refused_rather_than_answered_wrongly() { assert_eq!(skill.sigma(), 0.0); assert!(skill.mu().is_finite()); - // `predict_quality` previously PANICKED here, out of a method that returns + // `quality` previously PANICKED here, out of a method that returns // `Result`: the contrast covariance is exactly singular when beta is zero // and every skill is a point mass. all_predictions!(h, |name: &str, r: Result<(), InferenceError>| { diff --git a/tests/quality.rs b/tests/quality.rs index 15b76a6..19e9a64 100644 --- a/tests/quality.rs +++ b/tests/quality.rs @@ -110,11 +110,8 @@ fn history_predict_quality_supports_three_teams() { h.record_winner(&"b", &"c", 2).unwrap(); let _ = h.converge().unwrap(); - let q = h.predict_quality(&[&[&"a"], &[&"b"], &[&"c"]]).unwrap(); - assert!( - q.is_finite(), - "3-team predict_quality must be finite, got {q}" - ); + let q = h.quality(&[&[&"a"], &[&"b"], &[&"c"]]).unwrap(); + assert!(q.is_finite(), "3-team quality must be finite, got {q}"); assert!((0.0..=1.0).contains(&q), "out of range: {q}"); } diff --git a/tests/validation.rs b/tests/validation.rs index eff6dfa..49d1ce2 100644 --- a/tests/validation.rs +++ b/tests/validation.rs @@ -139,7 +139,7 @@ fn ingestion_rejects_a_tie_without_a_draw_probability() { ); } -/// `Outcome::scores_with_sigma` documents that a non-positive sigma is +/// `Outcome::scores_with_noise` documents that a non-positive sigma is /// accepted at construction and rejected at ingestion. #[test] fn ingestion_rejects_a_non_positive_per_event_score_sigma() { @@ -152,7 +152,7 @@ fn ingestion_rejects_a_non_positive_per_event_score_sigma() { Team::with_members([Member::new("a")]), Team::with_members([Member::new("b")]), ], - outcome: Outcome::scores_with_sigma([21.0, 9.0], sigma), + outcome: Outcome::scores_with_noise([21.0, 9.0], sigma), }]) .expect_err("a non-positive per-event sigma must be rejected"); assert!(