Six of fifteen variants carried a `&'static str` discriminator, about
thirty magic strings between them, and the only thing a caller could do
with one was print it. Four new enums replace them:
Parameter 13 variants, replacing 9 strings in InvalidParameter
Shape 4 variants, replacing 10 in MismatchedShape
OutcomeKind 2 variants, replacing WrongOutcomeKind's three fields
CompetitorField 2 variants, replacing ConflictingCompetitorConfig's
`InvalidProbability` folds into `InvalidParameter` as
`Parameter::PDraw`. It was a bespoke variant for one scalar while every
other scalar shared `InvalidParameter`, and it omitted the parameter
name — so the same parameter had two mechanisms.
`JointUnavailable { reason: &'static str }` splits into `EmptyHistory`,
`JointRequiresScoredEvents` and `NotPositiveDefinite`. The three are
conditions a caller branches on differently — add events, use
`predict_win_probabilities`, or reconsider the priors — and telling them
apart used to mean string-matching English. One test already proved the
distinction was load-bearing: the blanket conversion mapped the
empty-history case onto the ranked one and `an_empty_history_has_no_joint`
caught it immediately.
`NonFiniteResult` splits into `NonFiniteStep { context, step }` and
`NonFiniteSkill { mu, sigma }`. One `step: (f64, f64)` field was
carrying a sweep step from `converge` and a skill's own moments from a
prediction — two situations in one variant, and a field name that could
only be right for one of them.
`InvalidParameter { name: "beta with point-mass skills" }` becomes
`NoPerformanceVariance`. It was never a parameter out of range: both
values are individually valid and it is their combination that leaves
nothing varying.
Three `Display` impls did not meet the standard the others set, and the
typed data is what makes fixing them possible:
before drift variance is invalid: NaN
after drift variance must be finite and non-negative (got NaN)
before kinds: expected length 3, got 2
after the outcome describes a different number of teams than the
event has: expected 3, got 2
before Game::ranked: expected Outcome::Ranked, got Outcome::Scored
after expected Outcome::Ranked, got Outcome::Scored; call
Game::scored for a scored outcome
`Parameter::range()` states each parameter's actual bounds, which no
`&'static str` name could have. `error::message_tests` renders every one
and asserts each is a sentence rather than a label, and that the three
above now carry a range or a next step.
The four internal `MismatchedShape` kinds — `results`, `times`, `kinds`,
and the weights array — collapse to `Shape::Internal`, whose `Display`
says plainly that reaching it is a bug in this crate. They are checks on
`add_events_with_prior`'s own parallel arrays and are unreachable
through the public API; they stay checked rather than becoming
`debug_assert!`s, because release is where this crate's defects hide.
Closes #74.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
401 lines
13 KiB
Rust
401 lines
13 KiB
Rust
//! Input validation must hold in **release**, where `debug_assert!` is gone.
|
|
//!
|
|
//! The engine guards itself with `debug_assert!`, which documents invariants
|
|
//! but vanishes in the profile users actually ship. Anything reachable from the
|
|
//! public API has to be rejected with an `InferenceError` instead, at the
|
|
//! boundary, rather than becoming NaN or an out-of-bounds panic deep inside
|
|
//! `run_chain`.
|
|
//!
|
|
//! `GameOptions` and `ConvergenceOptions` both have public fields, so the
|
|
//! eager asserts on `HistoryBuilder` do not cover the `Game` constructors —
|
|
//! a caller can build the options struct directly.
|
|
|
|
use smallvec::smallvec;
|
|
use trueskill_tt::{
|
|
ConstantDrift, ConvergenceOptions, Event, Game, GameOptions, Gaussian, History, InferenceError,
|
|
Member, Outcome, Rating, Team,
|
|
};
|
|
|
|
type R = Rating<i64, ConstantDrift>;
|
|
|
|
fn rating() -> R {
|
|
R::new(
|
|
Gaussian::from_ms(25.0, 25.0 / 3.0),
|
|
25.0 / 6.0,
|
|
ConstantDrift::new(0.0),
|
|
)
|
|
}
|
|
|
|
fn options_with_alpha(alpha: f64) -> GameOptions {
|
|
GameOptions {
|
|
convergence: ConvergenceOptions {
|
|
alpha,
|
|
..ConvergenceOptions::default()
|
|
},
|
|
..GameOptions::default()
|
|
}
|
|
}
|
|
|
|
/// `alpha == 0.0` leaves every EP update unapplied, so inference silently
|
|
/// returns the priors — the worst possible failure, since the output looks
|
|
/// entirely reasonable.
|
|
#[test]
|
|
fn ranked_rejects_a_zero_damping_factor() {
|
|
let (a, b) = (rating(), rating());
|
|
let err = Game::<i64, _>::ranked(
|
|
&[&[a], &[b]],
|
|
Outcome::winner(0, 2),
|
|
&options_with_alpha(0.0),
|
|
)
|
|
.expect_err("alpha = 0 must be rejected");
|
|
assert!(
|
|
matches!(
|
|
err,
|
|
InferenceError::InvalidParameter {
|
|
parameter: trueskill_tt::Parameter::Alpha,
|
|
..
|
|
}
|
|
),
|
|
"got {err:?}"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn ranked_rejects_an_out_of_range_damping_factor() {
|
|
let (a, b) = (rating(), rating());
|
|
for alpha in [-0.5, 1.5, f64::NAN] {
|
|
let err = Game::<i64, _>::ranked(
|
|
&[&[a], &[b]],
|
|
Outcome::winner(0, 2),
|
|
&options_with_alpha(alpha),
|
|
)
|
|
.expect_err("alpha out of (0, 1] must be rejected");
|
|
assert!(
|
|
matches!(
|
|
err,
|
|
InferenceError::InvalidParameter {
|
|
parameter: trueskill_tt::Parameter::Alpha,
|
|
..
|
|
}
|
|
),
|
|
"alpha={alpha}: got {err:?}"
|
|
);
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn scored_rejects_a_bad_damping_factor() {
|
|
let (a, b) = (rating(), rating());
|
|
let err = Game::<i64, _>::scored(
|
|
&[&[a], &[b]],
|
|
Outcome::scores([21.0, 9.0]),
|
|
&options_with_alpha(0.0),
|
|
)
|
|
.expect_err("alpha = 0 must be rejected");
|
|
assert!(
|
|
matches!(
|
|
err,
|
|
InferenceError::InvalidParameter {
|
|
parameter: trueskill_tt::Parameter::Alpha,
|
|
..
|
|
}
|
|
),
|
|
"got {err:?}"
|
|
);
|
|
}
|
|
|
|
/// Already covered by `Game::ranked`, asserted here so the release-mode
|
|
/// guarantee is stated in one place.
|
|
#[test]
|
|
fn ranked_rejects_an_out_of_range_draw_probability() {
|
|
let (a, b) = (rating(), rating());
|
|
for p_draw in [-0.5, 1.0, 1.5] {
|
|
let options = GameOptions {
|
|
p_draw,
|
|
..GameOptions::default()
|
|
};
|
|
assert!(
|
|
Game::<i64, _>::ranked(&[&[a], &[b]], Outcome::winner(0, 2), &options).is_err(),
|
|
"p_draw={p_draw} must be rejected"
|
|
);
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn scored_rejects_a_non_positive_noise() {
|
|
let (a, b) = (rating(), rating());
|
|
for score_sigma in [0.0, -1.0, f64::NAN] {
|
|
let options = GameOptions {
|
|
score_sigma,
|
|
..GameOptions::default()
|
|
};
|
|
assert!(
|
|
Game::<i64, _>::scored(&[&[a], &[b]], Outcome::scores([21.0, 9.0]), &options).is_err(),
|
|
"score_sigma={score_sigma} must be rejected"
|
|
);
|
|
}
|
|
}
|
|
|
|
/// A tie with no draw probability makes the truncation margin zero and the
|
|
/// two-sided update evaluate 0/0. Ingestion must refuse it.
|
|
#[test]
|
|
fn ingestion_rejects_a_tie_without_a_draw_probability() {
|
|
let mut h = History::builder().p_draw(0.0).build();
|
|
let err = h
|
|
.add_events(vec![Event {
|
|
time: 0,
|
|
teams: smallvec![
|
|
Team::with_members([Member::new("a")]),
|
|
Team::with_members([Member::new("b")]),
|
|
],
|
|
outcome: Outcome::draw(2),
|
|
}])
|
|
.expect_err("a tie with p_draw = 0 must be rejected");
|
|
assert!(
|
|
matches!(err, InferenceError::TieWithoutDrawProbability { .. }),
|
|
"got {err:?}"
|
|
);
|
|
}
|
|
|
|
/// `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() {
|
|
for sigma in [0.0, -1.0, f64::NAN] {
|
|
let mut h = History::builder().build();
|
|
let err = h
|
|
.add_events(vec![Event {
|
|
time: 0,
|
|
teams: smallvec![
|
|
Team::with_members([Member::new("a")]),
|
|
Team::with_members([Member::new("b")]),
|
|
],
|
|
outcome: Outcome::scores_with_noise([21.0, 9.0], sigma),
|
|
}])
|
|
.expect_err("a non-positive per-event sigma must be rejected");
|
|
assert!(
|
|
matches!(err, InferenceError::InvalidParameter { .. }),
|
|
"sigma={sigma}: got {err:?}"
|
|
);
|
|
}
|
|
}
|
|
|
|
/// Per-team weights must match that team's membership. The top-level length
|
|
/// checks in ingestion do not cover the inner dimension.
|
|
#[test]
|
|
fn ingestion_rejects_weights_that_do_not_match_their_team() {
|
|
let mut h = History::builder().build();
|
|
let mut team = Team::with_members([Member::new("a"), Member::new("b")]);
|
|
team.members[0].weight = 1.0;
|
|
|
|
let err = h
|
|
.event(0)
|
|
.team(["a", "b"])
|
|
.team(["c"])
|
|
// Three weights for a two-member team.
|
|
.weights([1.0, 1.0, 1.0])
|
|
.winner(0)
|
|
.commit()
|
|
.expect_err("a weight/member length mismatch must be rejected");
|
|
assert!(
|
|
matches!(err, InferenceError::MismatchedShape { .. }),
|
|
"got {err:?}"
|
|
);
|
|
}
|
|
|
|
/// `mu`, `sigma` and `beta` were the last unvalidated setters on
|
|
/// `HistoryBuilder`, next to `p_draw`, `score_sigma` and `convergence`, which
|
|
/// all assert eagerly.
|
|
///
|
|
/// Two of the rejected values are the quiet kind. A negative `sigma` or `beta`
|
|
/// enters inference only as its square, so it produced bit-identical results
|
|
/// to the positive value — the sign was dropped without comment.
|
|
mod builder_parameters {
|
|
use trueskill_tt::History;
|
|
|
|
#[test]
|
|
#[should_panic(expected = "mu must be finite")]
|
|
fn a_non_finite_mu_is_rejected() {
|
|
let _ = History::builder().mu(f64::NAN);
|
|
}
|
|
|
|
#[test]
|
|
#[should_panic(expected = "sigma must be finite and positive")]
|
|
fn a_zero_sigma_is_rejected() {
|
|
let _ = History::builder().sigma(0.0);
|
|
}
|
|
|
|
#[test]
|
|
#[should_panic(expected = "sigma must be finite and positive")]
|
|
fn a_negative_sigma_is_rejected() {
|
|
let _ = History::builder().sigma(-8.33);
|
|
}
|
|
|
|
#[test]
|
|
#[should_panic(expected = "sigma must be finite and positive")]
|
|
fn an_infinite_sigma_is_rejected() {
|
|
let _ = History::builder().sigma(f64::INFINITY);
|
|
}
|
|
|
|
#[test]
|
|
#[should_panic(expected = "beta must be finite and non-negative")]
|
|
fn a_negative_beta_is_rejected() {
|
|
let _ = History::builder().beta(-4.17);
|
|
}
|
|
|
|
#[test]
|
|
#[should_panic(expected = "beta must be finite and non-negative")]
|
|
fn a_non_finite_beta_is_rejected() {
|
|
let _ = History::builder().beta(f64::NAN);
|
|
}
|
|
|
|
/// Zero beta is deliberately allowed: performance is then exactly skill.
|
|
/// It has to reach a different fit than a positive beta, or "allowed"
|
|
/// would just mean "not checked".
|
|
#[test]
|
|
fn a_zero_beta_is_allowed_and_changes_the_fit() {
|
|
let fit = |beta: f64| {
|
|
let mut h = History::builder()
|
|
.mu(25.0)
|
|
.sigma(25.0 / 3.0)
|
|
.beta(beta)
|
|
.build();
|
|
h.record_winner(&"a", &"b", 1).unwrap();
|
|
let _ = h.converge().unwrap();
|
|
h.current_skill(&"a").unwrap()
|
|
};
|
|
let zero = fit(0.0);
|
|
let positive = fit(25.0 / 6.0);
|
|
// `variance` rather than `pi`: the natural parameters are the crate's
|
|
// internal representation and no longer public. It is the same
|
|
// quantity inverted, so a finite positive precision is a finite
|
|
// positive variance.
|
|
assert!(zero.variance().is_finite() && zero.variance() > 0.0);
|
|
assert!(
|
|
(zero.variance() - positive.variance()).abs() > 1e-6,
|
|
"zero beta must not merely be ignored: {zero:?} vs {positive:?}"
|
|
);
|
|
}
|
|
}
|
|
|
|
/// The constructors below `HistoryBuilder`, which 0.8.0's validation did not
|
|
/// reach.
|
|
///
|
|
/// `sigma`, `beta` and `gamma` all enter inference only as squares, so a
|
|
/// negative value behaves as its absolute value and the sign vanishes without
|
|
/// comment. Measured before these guards: `from_ms(25.0, -8.33)` and
|
|
/// `Rating::new(_, -4.17, _)` returned results bit identical to their positive
|
|
/// counterparts, and `Rating::new(_, NaN, _)` reached `Game::ranked`, which
|
|
/// returned `Ok` carrying `Gaussian { pi: NaN, tau: NaN }`.
|
|
mod constructor_parameters {
|
|
use trueskill_tt::{ConstantDrift, Gaussian, History, InferenceError, Rating};
|
|
|
|
#[test]
|
|
#[should_panic(expected = "sigma must not be negative")]
|
|
fn a_negative_sigma_is_rejected_by_from_ms() {
|
|
let _ = Gaussian::from_ms(25.0, -8.33);
|
|
}
|
|
|
|
/// NaN must pass, and that is deliberate: a broken fit produces a NaN
|
|
/// sigma and `converge` reports it as `NonFiniteResult`. Rejecting it here
|
|
/// would turn reporting into a panic inside inference.
|
|
#[test]
|
|
fn a_nan_sigma_passes_through_from_ms() {
|
|
let g = Gaussian::from_ms(25.0, f64::NAN);
|
|
// `sigma()` is NaN exactly when the precision is: it guards `pi <= 0`
|
|
// (reporting `inf`) and `pi == inf` (reporting `0.0`), so NaN survives
|
|
// only from a NaN precision.
|
|
assert!(g.sigma().is_nan());
|
|
}
|
|
|
|
#[test]
|
|
#[should_panic(expected = "beta must be finite and non-negative")]
|
|
fn a_negative_beta_is_rejected_by_rating_new() {
|
|
let _ =
|
|
Rating::<i64, ConstantDrift>::new(Gaussian::default(), -4.17, ConstantDrift::new(0.0));
|
|
}
|
|
|
|
#[test]
|
|
#[should_panic(expected = "beta must be finite and non-negative")]
|
|
fn a_nan_beta_is_rejected_by_rating_new() {
|
|
let _ = Rating::<i64, ConstantDrift>::new(
|
|
Gaussian::default(),
|
|
f64::NAN,
|
|
ConstantDrift::new(0.0),
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn a_zero_beta_is_accepted_by_rating_new() {
|
|
let _ =
|
|
Rating::<i64, ConstantDrift>::new(Gaussian::default(), 0.0, ConstantDrift::new(0.0));
|
|
}
|
|
|
|
/// `ConstantDrift` rejects at construction now that its field is private.
|
|
#[test]
|
|
#[should_panic(expected = "gamma must be finite and non-negative")]
|
|
fn a_negative_gamma_is_rejected_by_constant_drift_new() {
|
|
let _ = ConstantDrift::new(-0.0833);
|
|
}
|
|
|
|
#[test]
|
|
#[should_panic(expected = "gamma must be finite and non-negative")]
|
|
fn a_non_finite_gamma_is_rejected_by_constant_drift_new() {
|
|
let _ = ConstantDrift::new(f64::NAN);
|
|
}
|
|
|
|
#[test]
|
|
fn gamma_reads_back_what_was_given() {
|
|
assert_eq!(ConstantDrift::new(0.25).gamma(), 0.25);
|
|
assert_eq!(ConstantDrift::new(0.0).gamma(), 0.0);
|
|
}
|
|
|
|
/// `HistoryBuilder::drift` is generic and cannot inspect an arbitrary
|
|
/// `Drift`, so the check on the variance each competitor accumulates is
|
|
/// still needed — it is the only thing standing between a custom
|
|
/// implementation and a NaN fit. `ConstantDrift` can no longer reach it,
|
|
/// so this uses an implementation that can.
|
|
#[test]
|
|
fn a_custom_drift_returning_a_bad_variance_is_rejected_at_convergence() {
|
|
#[derive(Clone, Copy, Debug)]
|
|
struct BadDrift(f64);
|
|
|
|
impl trueskill_tt::Drift<i64> for BadDrift {
|
|
fn variance_delta(&self, _from: &i64, _to: &i64) -> f64 {
|
|
self.0
|
|
}
|
|
fn variance_for_elapsed(&self, _elapsed: i64) -> f64 {
|
|
self.0
|
|
}
|
|
}
|
|
|
|
for bad in [f64::NAN, f64::INFINITY, -1.0] {
|
|
let mut h = History::builder().drift(BadDrift(bad)).build();
|
|
h.record_winner(&"a", &"b", 1).unwrap();
|
|
h.record_winner(&"a", &"b", 5).unwrap();
|
|
let err = h.converge().unwrap_err();
|
|
assert!(
|
|
matches!(
|
|
err,
|
|
InferenceError::InvalidParameter {
|
|
parameter: trueskill_tt::Parameter::DriftVariance,
|
|
..
|
|
}
|
|
),
|
|
"drift {bad}: {err:?}"
|
|
);
|
|
}
|
|
}
|
|
|
|
/// An ordinary drift is untouched.
|
|
#[test]
|
|
fn an_ordinary_drift_still_converges() {
|
|
let mut h = History::builder()
|
|
.drift(ConstantDrift::new(25.0 / 300.0))
|
|
.build();
|
|
h.record_winner(&"a", &"b", 1).unwrap();
|
|
h.record_winner(&"a", &"b", 5).unwrap();
|
|
assert!(h.converge().unwrap().converged);
|
|
}
|
|
}
|