Files
trueskill-tt/tests/non_finite_results.rs
T
logaritmiskandClaude Opus 5 c65373f476 fix!: propagate NaN through the convergence reduction
`tuple_max` compared with a plain `>`, which is false against NaN, so a
NaN accumulator was replaced by the next finite delta. The fold runs over
`TimeSlice::posteriors()`, a HashMap, so whether a NaN survived to `step`
depended on per-process hash order.

Measured before, four competitors in one slice with one pathological
pair, same binary and input, 30 separate processes:

  16  Ok  converged=true, iterations=1, a = Gaussian { pi: NaN, tau: NaN }
  14  Err NonFiniteResult

After: 30/30 Err. A coin flip on whether a NaN fit was reported as an
error or as a successful, converged fit — inside the guard whose entire
purpose is "NaN is never convergence".

`f64::max` would not have fixed it. It also ignores NaN by design, which
is the same defect wearing a standard-library name, and a test pins that
we do not use it.

`Gaussian::delta` had to be fixed FIRST, and that ordering is the whole
subtlety. Two identical improper messages produced `(0.0, NaN)` — not
from `mu()`, which is guarded and returns 0.0, but from `inf - inf` in
the sigma component. That NaN is reachable in ordinary healthy inference:
once a pairing is more than about nine cavity-sigma apart the truncation
is a no-op and the chain compares one identity message against another.
Propagating NaN without fixing `delta` would therefore have turned
correct fits into NonFiniteResult errors. `delta` now answers the
identical-message case in natural space before touching the accessors.

My first version of the `delta` test asserted `mu()` was NaN. It is not;
the accessor guards `pi <= 0.0`. The test caught my own wrong premise,
and the doc comment is corrected to match.

BREAKING CHANGE: a fit that produced NaN in a non-final reduction
position previously returned `Ok` with `converged: true` and a NaN
posterior; it now returns `Err(NonFiniteResult)`. That was always the
documented intent.

Closes #58

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
2026-09-09 16:59:56 +02:00

166 lines
6.1 KiB
Rust

//! Inference must report numerical breakdown rather than call it convergence.
//!
//! The boundary rejects inputs that are *not numbers*, but finite inputs can
//! still overflow during inference — `beta.powi(2)` at 1e300 is infinite, and
//! infinity minus infinity is NaN. `NonFiniteResult` is the guard for that, and
//! it matters because the alternative is silent: NaN fails every comparison, so
//! a naive `step < epsilon` check reads a NaN step as *converged*.
//!
//! That is why the crate has `step_converged` / `step_is_finite` rather than
//! `!tuple_gt(..)`. These tests pin the guard from outside.
use smallvec::smallvec;
use trueskill_tt::{Event, Gaussian, History, InferenceError, Member, Outcome, Team};
fn scored_fit(
sigma: f64,
beta: f64,
score_sigma: f64,
scores: [f64; 2],
) -> Result<bool, InferenceError> {
let mut h = History::builder()
.mu(0.0)
.sigma(sigma)
.beta(beta)
.score_sigma(score_sigma)
.build();
h.add_events(vec![Event {
time: 1i64,
teams: smallvec![
Team::with_members([Member::new("a")]),
Team::with_members([Member::new("b")]),
],
outcome: Outcome::scores(scores),
}])?;
h.converge().map(|r| r.converged)
}
/// Every one of these is built from finite, individually legal parameters. The
/// overflow happens inside inference, which is exactly the case the boundary
/// checks cannot catch.
///
/// Matched rather than merely `is_err()`: an assertion that only checks "some
/// error" would keep passing if these started failing at the boundary for an
/// unrelated reason, and would then be testing nothing.
#[test]
fn overflow_during_inference_is_reported_not_hidden() {
let cases: [(&str, f64, f64, f64, [f64; 2]); 5] = [
("huge sigma", 1e300, 1.0, 1.0, [3.0, 1.0]),
("huge beta", 6.0, 1e300, 1.0, [3.0, 1.0]),
("tiny sigma", 1e-300, 1.0, 1.0, [3.0, 1.0]),
("tiny score_sigma", 6.0, 1.0, 1e-300, [3.0, 1.0]),
("huge scores", 6.0, 1.0, 1.0, [1e308, -1e308]),
];
for (name, sigma, beta, score_sigma, scores) in cases {
match scored_fit(sigma, beta, score_sigma, scores) {
Err(InferenceError::NonFiniteResult { context, step }) => {
assert_eq!(context, "History::converge", "{name}");
assert!(
!step.0.is_finite() || !step.1.is_finite(),
"{name}: reported NonFiniteResult with a finite step {step:?}"
);
}
other => panic!("{name}: expected NonFiniteResult, got {other:?}"),
}
}
}
/// The trap the invariant exists for: NaN fails every comparison, so a naive
/// `step < epsilon` test reads a NaN step as converged. A breakdown must never
/// come back as a successful fit.
#[test]
fn a_broken_fit_is_never_reported_as_converged() {
let mut h = History::builder().build();
h.add_events(vec![Event {
time: 1i64,
teams: smallvec![
Team::with_members([Member::new("a").with_prior(Gaussian::from_ms(1e300, 1e-300))]),
Team::with_members([Member::new("b")]),
],
outcome: Outcome::winner(0, 2),
}])
.unwrap();
let err = h.converge().unwrap_err();
assert!(
matches!(err, InferenceError::NonFiniteResult { .. }),
"a breakdown must not be reported as convergence: {err:?}"
);
// `converge_partial` must not launder it into an `Ok` either — the
// permissive path is permissive about *stopping short*, not about NaN.
let mut h2 = History::builder().build();
h2.add_events(vec![Event {
time: 1i64,
teams: smallvec![
Team::with_members([Member::new("a").with_prior(Gaussian::from_ms(1e300, 1e-300))]),
Team::with_members([Member::new("b")]),
],
outcome: Outcome::winner(0, 2),
}])
.unwrap();
assert!(matches!(
h2.converge_partial().unwrap_err(),
InferenceError::NonFiniteResult { .. }
));
}
/// The neighbouring case, so the tests above cannot pass by the fit simply
/// always failing: ordinary extreme-but-workable parameters still converge.
#[test]
fn merely_extreme_parameters_still_converge() {
assert!(scored_fit(1e6, 1.0, 1.0, [3.0, 1.0]).unwrap());
assert!(scored_fit(1e-6, 1.0, 1.0, [3.0, 1.0]).unwrap());
assert!(scored_fit(6.0, 1.0, 1e6, [3.0, 1.0]).unwrap());
assert!(scored_fit(6.0, 1.0, 1.0, [1e150, -1e150]).unwrap());
}
/// A NaN in one competitor must not be masked by a healthy competitor reduced
/// after it.
///
/// The convergence step is a fold over a `HashMap`, so which competitor is
/// reduced last is per-process hash order. Before the fix, `tuple_max` dropped
/// a NaN accumulator in favour of the next finite delta and this returned
/// `Ok(converged: true)` with a NaN posterior in **16 of 30 runs** on identical
/// input. Deterministic now, but note this test can only ever sample one hash
/// order per run — the ordering guarantee itself is pinned by
/// `tuple_max_propagates_a_nan_from_any_position` in the crate's unit tests.
#[test]
fn a_nan_competitor_is_not_masked_by_a_healthy_one() {
let mut h = History::builder()
.mu(0.0)
.sigma(6.0)
.beta(1.0)
.p_draw(0.1)
.build();
h.add_events(vec![
Event {
time: 1i64,
teams: smallvec![
Team::with_members([Member::new("a").with_prior(Gaussian::from_ms(0.0, 1e-200))]),
Team::with_members([Member::new("b")]),
],
outcome: Outcome::winner(0, 2),
},
// A healthy pair in the same slice, to be reduced alongside the NaN.
Event {
time: 1i64,
teams: smallvec![
Team::with_members([Member::new("c")]),
Team::with_members([Member::new("d")]),
],
outcome: Outcome::winner(0, 2),
},
])
.unwrap();
let err = h
.converge()
.expect_err("a NaN fit must never be reported as converged");
assert!(
matches!(err, InferenceError::NonFiniteResult { .. }),
"{err:?}"
);
}