Documentation (#78). Every item below was measured against the code rather than read: - `expected_information_gain` and `predict_ranking` had `# Errors` immediately followed by `# Preconditions`, with the error list stranded at the bottom of the latter — rustdoc rendered a BLANK Errors section on both. The heading now sits with its content. - `predict_outcome`, `predict_ranking` and the free `expected_information_gain` all omitted `GridTooCoarse`. - `predict_margin` claimed `JointUnavailable` "if the LATEST slice holds ranked events". Measured with an early ranked slice and a late scored one: it fails. The condition is *any* slice. - `add_events` documented three errors and can return five more; it also claimed a weights `MismatchedShape` that is unreachable through it, since weights arrive one-per-`Member`. That check belongs to `EventBuilder::weights`, and the doc now says so. - `converge` and `converge_partial` both omitted the drift-variance `InvalidParameter`. `History` gains a hand-written `Debug` (#76). Summarising, not exhaustive — a derived one would print every competitor's skill at every slice. It exists because without it a consumer cannot `#[derive(Debug)]` on any struct holding a `History`, which is how both known consumers store one. `#[non_exhaustive]` on all 17 `InferenceError` struct variants and on `Outcome::Scored` (#74). The enum carried the attribute; no variant did, so adding a field to any of them — and downstream construction of any of them — were both in the public contract. This crate added two variants in two days. The options structs are deliberately NOT sealed. `ConvergenceOptions` and `GameOptions` are constructed by struct literal at 65 sites of which only 8 use `..default()`, and specifying all three convergence fields is a natural complete statement rather than a partial one. That is a real trade-off rather than an oversight, and it is left as a decision on #74. Also spells `UnknownKeys::Reject` explicitly at both sites that wildcarded it. `#[non_exhaustive]` on your own enum gives no exhaustiveness safety net if you then match `_`. Sealing the variants pushed ten test sites from constructing errors to `matches!`, which is the better assertion anyway — an `assert_eq!` against a constructed error breaks whenever a field is added, which is the exact fragility the attribute exists to prevent. BREAKING CHANGE: `InferenceError`'s struct variants and `Outcome::Scored` are `#[non_exhaustive]` — downstream patterns need `..` and downstream construction is no longer possible. Refs #78, #76, #74 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
155 lines
4.5 KiB
Rust
155 lines
4.5 KiB
Rust
//! Stopping short of convergence is an error, not a flag on a success.
|
|
//!
|
|
//! A fit that hits `max_iter` is wrong by a little: every rating is finite,
|
|
//! the ordering looks sensible, and nothing in the numbers says they were
|
|
//! still moving. When that was `Ok` with `converged: false`, detecting it was
|
|
//! opt-in and `let _ = h.converge()` was the natural way to opt out — which is
|
|
//! how a real defect once hid in this crate's own suite.
|
|
|
|
use smallvec::smallvec;
|
|
use trueskill_tt::{
|
|
ConstantDrift, ConvergenceOptions, Event, History, InferenceError, Member, Outcome, Team,
|
|
};
|
|
|
|
type H = History<i64, ConstantDrift, trueskill_tt::NullObserver, &'static str>;
|
|
|
|
fn duel(a: &'static str, b: &'static str, t: i64) -> Event<i64, &'static str> {
|
|
Event {
|
|
time: t,
|
|
teams: smallvec![
|
|
Team::with_members([Member::new(a)]),
|
|
Team::with_members([Member::new(b)]),
|
|
],
|
|
outcome: Outcome::scores([3.0, 1.0]),
|
|
}
|
|
}
|
|
|
|
fn capped(max_iter: usize) -> H {
|
|
History::builder()
|
|
.mu(0.0)
|
|
.sigma(6.0)
|
|
.beta(1.0)
|
|
.score_sigma(2.0)
|
|
.drift(ConstantDrift::new(0.5))
|
|
.convergence(ConvergenceOptions {
|
|
max_iter,
|
|
epsilon: 1e-13,
|
|
alpha: 1.0,
|
|
})
|
|
.build()
|
|
}
|
|
|
|
fn fill(h: &mut H) {
|
|
h.add_events((1..=6).map(|t| duel("a", "b", t)).collect::<Vec<_>>())
|
|
.unwrap();
|
|
}
|
|
|
|
#[test]
|
|
fn hitting_the_cap_is_an_error() {
|
|
let mut h = capped(1);
|
|
fill(&mut h);
|
|
let err = h.converge().unwrap_err();
|
|
match err {
|
|
InferenceError::NotConverged {
|
|
iterations,
|
|
final_step,
|
|
epsilon,
|
|
..
|
|
} => {
|
|
assert_eq!(iterations, 1);
|
|
assert!(
|
|
final_step.0 > epsilon || final_step.1 > epsilon,
|
|
"{final_step:?}"
|
|
);
|
|
}
|
|
other => panic!("expected NotConverged, got {other:?}"),
|
|
}
|
|
}
|
|
|
|
/// The message has to name what to do about it, since the fit looks fine.
|
|
#[test]
|
|
fn the_error_says_how_to_fix_it() {
|
|
let mut h = capped(1);
|
|
fill(&mut h);
|
|
let text = h.converge().unwrap_err().to_string();
|
|
assert!(text.contains("did not converge in 1 iterations"), "{text}");
|
|
assert!(text.contains("max_iter"), "{text}");
|
|
assert!(text.contains("alpha"), "{text}");
|
|
}
|
|
|
|
/// The escape hatch: a deliberately capped fit is still reachable.
|
|
#[test]
|
|
fn converge_partial_returns_the_short_fit() {
|
|
let mut h = capped(1);
|
|
fill(&mut h);
|
|
let report = h.converge_partial().unwrap();
|
|
assert_eq!(report.iterations, 1);
|
|
assert!(!report.converged);
|
|
assert!(h.current_skill(&"a").is_some());
|
|
}
|
|
|
|
/// Both agree when the fit does converge, so the strict path costs nothing.
|
|
#[test]
|
|
fn the_two_agree_on_a_converged_fit() {
|
|
let mut strict = capped(20_000);
|
|
fill(&mut strict);
|
|
let a = strict.converge().unwrap();
|
|
|
|
let mut partial = capped(20_000);
|
|
fill(&mut partial);
|
|
let b = partial.converge_partial().unwrap();
|
|
|
|
assert!(a.converged && b.converged);
|
|
assert_eq!(a.iterations, b.iterations);
|
|
assert_eq!(a.final_step, b.final_step);
|
|
}
|
|
|
|
/// The default cap must be high enough that an ordinary history clears it.
|
|
/// At the old value of 30 this history stopped short and said nothing.
|
|
#[test]
|
|
fn the_default_cap_clears_an_ordinary_history() {
|
|
let mut h: History<i64, ConstantDrift, _, String> = History::builder()
|
|
.key_type::<String>()
|
|
.mu(0.0)
|
|
.sigma(6.0)
|
|
.beta(1.0)
|
|
.score_sigma(2.0)
|
|
.drift(ConstantDrift::new(0.05))
|
|
.build();
|
|
|
|
let mut events = Vec::new();
|
|
for t in 0..20i64 {
|
|
for j in 0..8usize {
|
|
let k = (t as usize) * 8 + j;
|
|
events.push(Event {
|
|
time: t,
|
|
teams: smallvec![
|
|
Team::with_members([Member::new(format!("p{}", k % 100))]),
|
|
Team::with_members([Member::new(format!("p{}", (k + 37) % 100))]),
|
|
],
|
|
outcome: Outcome::scores([3.0, 1.0]),
|
|
});
|
|
}
|
|
}
|
|
h.add_events(events).unwrap();
|
|
|
|
let report = h
|
|
.converge()
|
|
.expect("an ordinary history must converge by default");
|
|
assert!(
|
|
report.iterations > 30,
|
|
"needed {} sweeps",
|
|
report.iterations
|
|
);
|
|
assert!(report.iterations < trueskill_tt::ITERATIONS);
|
|
}
|
|
|
|
/// An empty history converges trivially rather than erroring.
|
|
#[test]
|
|
fn an_empty_history_converges() {
|
|
let mut h = capped(1);
|
|
let report = h.converge().unwrap();
|
|
assert!(report.converged);
|
|
assert_eq!(report.iterations, 0);
|
|
}
|