fix: reject ties without draw probability; never report NaN as converged
A tie with `p_draw == 0.0` produced NaN posteriors in release builds and `converge()` reported `converged: true`, because every comparison against NaN is false and `tuple_gt` therefore read NaN as "below epsilon". Two independent defects, fixed together: - Ingestion now rejects tied outcomes when the draw probability is zero, promoting the existing `debug_assert!` in `Game::ranked_with_arena` to a real `InferenceError::TieWithoutDrawProbability`. Validation sits in `add_events_with_prior`, the chokepoint every route reaches — including `record_draw`, which bypasses `Outcome` entirely. - `converge()` treats a non-finite step as failure and returns `InferenceError::NonFiniteResult` rather than claiming convergence. Also in this change: - `History::converge()` on an empty history returned a `usize` underflow panic from `0..len()-1`; it now short-circuits to a zero-iteration report. - `Outcome::scores_with_sigma` no longer panics on a non-positive sigma; the value is validated at ingestion so callers get an error instead. - `InferenceError` gains `WrongOutcomeKind`, replacing the misuse of `MismatchedShape` for variant mismatches (which rendered as the nonsense "expected length 0, got 0"), and is now `#[non_exhaustive]`. Note `Outcome::winner(w, n)` for n >= 3 ties every loser, so those events now require a positive `p_draw`. They previously returned NaN. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DnsaJg74eNSva3PJjK2eej
This commit is contained in:
@@ -0,0 +1,195 @@
|
||||
//! Degenerate, boundary, and error-path coverage.
|
||||
//!
|
||||
//! These run in both debug and release: the defects they pin were all
|
||||
//! guarded only by `debug_assert!`, so a debug-only suite never saw them.
|
||||
|
||||
use trueskill_tt::{
|
||||
ConstantDrift, ConvergenceOptions, Game, GameOptions, Gaussian, History, InferenceError,
|
||||
Outcome, Rating,
|
||||
};
|
||||
|
||||
type R = Rating<i64, ConstantDrift>;
|
||||
|
||||
fn rating() -> R {
|
||||
R::new(
|
||||
Gaussian::from_ms(25.0, 25.0 / 3.0),
|
||||
25.0 / 6.0,
|
||||
ConstantDrift(25.0 / 300.0),
|
||||
)
|
||||
}
|
||||
|
||||
fn assert_finite(g: Gaussian, what: &str) {
|
||||
assert!(
|
||||
g.mu().is_finite() && g.sigma().is_finite(),
|
||||
"{what} must be finite, got mu={} sigma={}",
|
||||
g.mu(),
|
||||
g.sigma()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn record_draw_without_draw_probability_is_rejected() {
|
||||
let mut h = History::default();
|
||||
let err = h.record_draw(&"a", &"b", 1).unwrap_err();
|
||||
assert!(matches!(
|
||||
err,
|
||||
InferenceError::TieWithoutDrawProbability { .. }
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn builder_draw_without_draw_probability_is_rejected() {
|
||||
let mut h = History::default();
|
||||
let err = h
|
||||
.event(1)
|
||||
.team(["a"])
|
||||
.team(["b"])
|
||||
.draw()
|
||||
.commit()
|
||||
.unwrap_err();
|
||||
assert!(matches!(
|
||||
err,
|
||||
InferenceError::TieWithoutDrawProbability { .. }
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn draw_with_positive_draw_probability_is_finite() {
|
||||
let mut h = History::builder().p_draw(0.25).build();
|
||||
h.record_draw(&"a", &"b", 1).unwrap();
|
||||
let report = h.converge().unwrap();
|
||||
|
||||
assert_finite(h.current_skill("a").unwrap(), "drawn competitor skill");
|
||||
assert_finite(h.current_skill("b").unwrap(), "drawn competitor skill");
|
||||
assert!(report.log_evidence.is_finite());
|
||||
assert!(report.converged);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn game_ranked_rejects_tie_without_draw_probability() {
|
||||
let a = [rating()];
|
||||
let b = [rating()];
|
||||
let teams: Vec<&[R]> = vec![&a, &b];
|
||||
let err = Game::ranked(&teams, Outcome::draw(2), &GameOptions::default()).unwrap_err();
|
||||
assert!(matches!(
|
||||
err,
|
||||
InferenceError::TieWithoutDrawProbability { .. }
|
||||
));
|
||||
}
|
||||
|
||||
/// `Outcome::winner(w, n)` ties every loser, so any n >= 3 free-for-all hits
|
||||
/// the tie path even though the caller never asked for a draw.
|
||||
#[test]
|
||||
fn winner_of_three_or_more_requires_draw_probability() {
|
||||
let a = [rating()];
|
||||
let b = [rating()];
|
||||
let c = [rating()];
|
||||
let teams: Vec<&[R]> = vec![&a, &b, &c];
|
||||
|
||||
let err = Game::ranked(&teams, Outcome::winner(0, 3), &GameOptions::default()).unwrap_err();
|
||||
assert!(matches!(
|
||||
err,
|
||||
InferenceError::TieWithoutDrawProbability { .. }
|
||||
));
|
||||
|
||||
let opts = GameOptions {
|
||||
p_draw: 0.1,
|
||||
..GameOptions::default()
|
||||
};
|
||||
let game = Game::ranked(&teams, Outcome::winner(0, 3), &opts).unwrap();
|
||||
for team in game.posteriors() {
|
||||
for skill in team {
|
||||
assert_finite(skill, "3-team winner posterior");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn full_ranking_without_ties_needs_no_draw_probability() {
|
||||
let a = [rating()];
|
||||
let b = [rating()];
|
||||
let c = [rating()];
|
||||
let teams: Vec<&[R]> = vec![&a, &b, &c];
|
||||
let game = Game::ranked(&teams, Outcome::ranking([0, 1, 2]), &GameOptions::default()).unwrap();
|
||||
|
||||
for team in game.posteriors() {
|
||||
for skill in team {
|
||||
assert_finite(skill, "strict ranking posterior");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_history_converges_trivially() {
|
||||
let mut h = History::default();
|
||||
let report = h.converge().unwrap();
|
||||
assert_eq!(report.iterations, 0);
|
||||
assert!(report.converged);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_event_stream_then_converge() {
|
||||
let mut h = History::default();
|
||||
h.add_events(std::iter::empty()).unwrap();
|
||||
let report = h.converge().unwrap();
|
||||
assert_eq!(report.iterations, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_history_queries_do_not_panic() {
|
||||
let h = History::default();
|
||||
assert!(h.learning_curves().is_empty());
|
||||
assert!(h.learning_curve("nobody").is_empty());
|
||||
assert!(h.current_skill("nobody").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn single_event_history_converges() {
|
||||
let mut h = History::default();
|
||||
h.record_winner(&"a", &"b", 1).unwrap();
|
||||
let report = h.converge().unwrap();
|
||||
assert!(report.converged);
|
||||
assert_finite(h.current_skill("a").unwrap(), "single-event skill");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn scored_event_rejects_non_positive_sigma() {
|
||||
let mut h = History::builder().score_sigma(2.0).build();
|
||||
let err = h
|
||||
.event(1)
|
||||
.team(["a"])
|
||||
.team(["b"])
|
||||
.scores_with_sigma([3.0, 1.0], f64::NAN)
|
||||
.commit()
|
||||
.unwrap_err();
|
||||
assert!(matches!(
|
||||
err,
|
||||
InferenceError::InvalidParameter {
|
||||
name: "score_sigma",
|
||||
..
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn convergence_reports_are_finite_across_many_teams() {
|
||||
let opts = GameOptions {
|
||||
p_draw: 0.1,
|
||||
convergence: ConvergenceOptions::default(),
|
||||
..GameOptions::default()
|
||||
};
|
||||
let holders: Vec<[R; 1]> = (0..12).map(|_| [rating()]).collect();
|
||||
let teams: Vec<&[R]> = holders.iter().map(|t| t.as_slice()).collect();
|
||||
let game = Game::ranked(&teams, Outcome::ranking(0..12), &opts).unwrap();
|
||||
|
||||
assert!(
|
||||
game.log_evidence().is_finite(),
|
||||
"12-team log-evidence must be finite, got {}",
|
||||
game.log_evidence()
|
||||
);
|
||||
for team in game.posteriors() {
|
||||
for skill in team {
|
||||
assert_finite(skill, "12-team posterior");
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user