From 061c481aad55ba31ded9e2e41f7d64f85dd7820c Mon Sep 17 00:00:00 2001 From: Anders Olsson Date: Thu, 10 Sep 2026 07:31:31 +0200 Subject: [PATCH] refactor!: typed discriminators for InferenceError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ --- README.md | 2 +- src/acquisition.rs | 10 +- src/convergence.rs | 4 +- src/drift.rs | 2 +- src/error.rs | 423 ++++++++++++++++++++++++++++----- src/event_builder.rs | 2 +- src/game.rs | 25 +- src/gaussian.rs | 4 +- src/history.rs | 128 +++++----- src/lib.rs | 2 +- src/outcome.rs | 2 +- tests/competitor_config.rs | 5 +- tests/degenerate_inputs.rs | 4 +- tests/drift_scale.rs | 6 +- tests/event_builder_members.rs | 4 +- tests/game.rs | 10 +- tests/ingestion_shape.rs | 16 +- tests/joint_handle.rs | 8 +- tests/non_finite_results.rs | 8 +- tests/prediction_guards.rs | 6 +- tests/registration.rs | 14 +- tests/validation.rs | 26 +- 22 files changed, 528 insertions(+), 183 deletions(-) diff --git a/README.md b/README.md index 798f859..7f21482 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ for everything that accumulates. ## `converge` is strict `converge` returns `Err(NotConverged)` if the sweep hits `max_iter` with the -step still above `epsilon`, and `Err(NonFiniteResult)` if a sweep produces NaN. +step still above `epsilon`, and `Err(NonFiniteStep)` if a sweep produces NaN. It used to return `Ok` with `converged: false`, which was the worst available shape. A fit that stops short is *wrong by a little*: every posterior is finite, diff --git a/src/acquisition.rs b/src/acquisition.rs index 3dd8822..724783f 100644 --- a/src/acquisition.rs +++ b/src/acquisition.rs @@ -123,7 +123,7 @@ fn u_minus_ln1p(u: f64) -> f64 { /// - `EmptyTeam` if any team has no members. /// - `TooManyTeams` if the outcome space is too large to enumerate; see /// [`MAX_PREDICTED_TEAMS`](crate::MAX_PREDICTED_TEAMS). -/// - `InvalidProbability` if `options.p_draw` is outside `[0.0, 1.0)`. +/// - `InvalidParameter` for a `p_draw` outside `[0.0, 1.0)`. /// - `GridTooCoarse` when the performance sigmas are too far apart to /// integrate on one grid. This comes from `outcome_distribution`, which runs /// before any inference — so it is not covered by "anything `Game::ranked` @@ -144,7 +144,8 @@ pub fn expected_information_gain>( }); } if !(0.0..1.0).contains(&options.p_draw) { - return Err(InferenceError::InvalidProbability { + return Err(InferenceError::InvalidParameter { + parameter: crate::Parameter::PDraw, value: options.p_draw, }); } @@ -367,7 +368,10 @@ mod tests { )); assert!(matches!( expected_information_gain(&[&a, &a], &options(1.5)), - Err(InferenceError::InvalidProbability { .. }) + Err(InferenceError::InvalidParameter { + parameter: crate::Parameter::PDraw, + .. + }) )); } diff --git a/src/convergence.rs b/src/convergence.rs index c0168cc..12d39d9 100644 --- a/src/convergence.rs +++ b/src/convergence.rs @@ -66,13 +66,13 @@ impl ConvergenceOptions { pub(crate) fn validate(&self) -> Result<(), crate::InferenceError> { if !(self.alpha > 0.0 && self.alpha <= 1.0) { return Err(crate::InferenceError::InvalidParameter { - name: "alpha", + parameter: crate::Parameter::Alpha, value: self.alpha, }); } if self.epsilon.is_nan() || self.epsilon < 0.0 { return Err(crate::InferenceError::InvalidParameter { - name: "epsilon", + parameter: crate::Parameter::Epsilon, value: self.epsilon, }); } diff --git a/src/drift.rs b/src/drift.rs index d4bf247..eb69f0f 100644 --- a/src/drift.rs +++ b/src/drift.rs @@ -35,7 +35,7 @@ pub trait Drift: Copy + Debug + Send + Sync { /// `variance_for_elapsed` would have been worse: it runs inside the sweep, so a /// construction-time mistake would panic mid-inference — and `Gaussian::from_ms` /// is a worked example of why that is the wrong place for a guard, where -/// rejecting NaN turned the `NonFiniteResult` reporting path into a crash. +/// rejecting NaN turned the `NonFiniteStep` reporting path into a crash. /// /// So [`ConstantDrift::new`] is the only way in, and it checks. Read the value /// back with [`ConstantDrift::gamma`]. diff --git a/src/error.rs b/src/error.rs index 2273032..fc0cbba 100644 --- a/src/error.rs +++ b/src/error.rs @@ -39,6 +39,182 @@ pub enum UnknownKeys { Prior, } +/// Which scalar an [`InferenceError::InvalidParameter`] is about. +/// +/// A typed discriminator rather than a `&'static str`, so a caller can branch +/// on it and `Display` can state each parameter's actual valid range. Nine +/// distinct strings used to flow through this position, and the only thing a +/// caller could do with one was print it. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[non_exhaustive] +pub enum Parameter { + /// Prior mean skill. Must be finite. + Mu, + /// Prior standard deviation. Must be finite and strictly positive. + Sigma, + /// Performance noise. Must be finite and non-negative. + Beta, + /// Draw probability. Must be in `[0.0, 1.0)`. + PDraw, + /// Observation noise on a score margin. Must be finite and strictly + /// positive. + ScoreSigma, + /// EP damping factor. Must be in `(0.0, 1.0]`. + Alpha, + /// Convergence threshold. Must be non-negative and not NaN. + Epsilon, + /// A competitor's multiplier on the drift variance. Must be finite and + /// non-negative. + DriftScale, + /// The variance a [`Drift`](crate::Drift) implementation actually produced + /// for a span. Must be finite and non-negative — checked because a custom + /// implementation is the one thing no constructor can validate up front. + DriftVariance, + /// A per-member weight on an event. Must be finite. + Weight, + /// A team's score on a scored event. Must be finite. + Score, + /// A team's rank on a ranked event. Must be finite. + Rank, + /// The winning team's index, as given to `Outcome::winner`. Must be less + /// than the team count. + WinnerIndex, +} + +impl Parameter { + /// The range this parameter must lie in, for the `Display` message. + fn range(self) -> &'static str { + match self { + Self::Mu => "must be finite", + Self::Sigma => "must be finite and strictly positive", + Self::Beta => "must be finite and non-negative", + Self::PDraw => "must be in [0.0, 1.0)", + Self::ScoreSigma => "must be finite and strictly positive", + Self::Alpha => "must be in (0.0, 1.0]", + Self::Epsilon => "must be non-negative and not NaN", + Self::DriftScale => "must be finite and non-negative", + Self::DriftVariance => "must be finite and non-negative", + Self::Weight => "must be finite", + Self::Score => "must be finite", + Self::Rank => "must be finite", + Self::WinnerIndex => "must be less than the number of teams", + } + } +} + +impl std::fmt::Display for Parameter { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let name = match self { + Self::Mu => "mu", + Self::Sigma => "sigma", + Self::Beta => "beta", + Self::PDraw => "p_draw", + Self::ScoreSigma => "score_sigma", + Self::Alpha => "alpha", + Self::Epsilon => "epsilon", + Self::DriftScale => "drift_scale", + Self::DriftVariance => "drift variance", + Self::Weight => "weight", + Self::Score => "score", + Self::Rank => "rank", + Self::WinnerIndex => "winner index", + }; + f.write_str(name) + } +} + +/// Which two lengths an [`InferenceError::MismatchedShape`] found disagreeing. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[non_exhaustive] +pub enum Shape { + /// The outcome describes a different number of teams than the event has. + OutcomeVsTeams, + /// A per-member weight list does not match the team's membership. + Weights, + /// A call that takes a fixed number of teams got a different number. + Teams, + /// One of `add_events_with_prior`'s parallel arrays disagreed with the + /// others. + /// + /// Not reachable through the public API — the arrays are built together at + /// the ingestion chokepoint. Kept as a checked error rather than a + /// `debug_assert!` so it also holds in release, which is where this + /// crate's defects have tended to hide. + Internal, +} + +impl std::fmt::Display for Shape { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let what = match self { + Self::OutcomeVsTeams => { + "the outcome describes a different number of teams than the event has" + } + Self::Weights => "the weight list does not match the team's membership", + Self::Teams => "this call takes a fixed number of teams", + Self::Internal => { + "an internal array disagreed with its siblings (this is a bug in trueskill-tt)" + } + }; + f.write_str(what) + } +} + +/// Which [`Outcome`](crate::Outcome) variant a call found or wanted. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[non_exhaustive] +pub enum OutcomeKind { + /// [`Outcome::Ranked`](crate::Outcome::Ranked): an ordinal finish. + Ranked, + /// [`Outcome::Scored`](crate::Outcome::Scored): continuous scores. + Scored, +} + +impl OutcomeKind { + /// The call that takes this kind, for the `Display` message. + fn constructor(self) -> &'static str { + match self { + Self::Ranked => "Game::ranked", + Self::Scored => "Game::scored", + } + } + + /// The adjective form, for prose. + fn adjective(self) -> &'static str { + match self { + Self::Ranked => "ranked", + Self::Scored => "scored", + } + } +} + +impl std::fmt::Display for OutcomeKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::Ranked => "Outcome::Ranked", + Self::Scored => "Outcome::Scored", + }) + } +} + +/// Which piece of per-competitor configuration was declared twice. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[non_exhaustive] +pub enum CompetitorField { + /// The starting skill distribution. + Prior, + /// The multiplier on the drift variance. + DriftScale, +} + +impl std::fmt::Display for CompetitorField { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::Prior => "prior", + Self::DriftScale => "drift_scale", + }) + } +} + /// Every way ingestion, inference or prediction can refuse to answer. /// /// The crate reports rather than repairs. An input it cannot represent, a fit @@ -56,9 +232,8 @@ pub enum InferenceError { /// Expected and actual lengths of some array-shaped input differ. #[non_exhaustive] MismatchedShape { - /// Which input disagreed, as a short label — `"ranks vs teams"`, - /// `"weights"`, `"times"`. - kind: &'static str, + /// Which pair of lengths disagreed. + shape: Shape, /// The length it had to have, taken from whatever it must line up with /// (usually the event's team count). expected: usize, @@ -68,28 +243,18 @@ pub enum InferenceError { /// An `Outcome` of the wrong variant was supplied for the requested inference. #[non_exhaustive] WrongOutcomeKind { - /// The call that rejected the outcome, e.g. `"Game::ranked"`. - context: &'static str, - /// The [`Outcome`](crate::Outcome) variant that call needs, by name. - expected: &'static str, - /// The variant actually supplied, by name. - got: &'static str, - }, - /// A probability value is outside `[0, 1]`. - #[non_exhaustive] - InvalidProbability { - /// The value supplied, as it fell outside `[0, 1]`. Today only - /// `p_draw` reaches here. - value: f64, + /// The variant the call needs. + expected: OutcomeKind, + /// The variant actually supplied. + got: OutcomeKind, }, /// A scalar parameter is outside its valid range. #[non_exhaustive] InvalidParameter { - /// The parameter, spelled as the API spells it — `"alpha"`, - /// `"epsilon"`, `"score_sigma"`, `"drift_scale"`, `"drift variance"`. - name: &'static str, - /// The value supplied for it. Out of that parameter's range, or NaN, - /// which fails every range comparison and is rejected on that basis. + /// Which parameter. `Display` states its valid range. + parameter: Parameter, + /// The value supplied for it: outside that range, or NaN, which fails + /// every range comparison and is rejected on that basis. value: f64, }, /// An event contains tied teams, but the draw probability is zero. @@ -130,20 +295,44 @@ pub enum InferenceError { /// The threshold both components of `final_step` had to reach. epsilon: f64, }, - /// Inference produced a non-finite value (NaN or infinity). + /// A convergence sweep produced a non-finite step. /// - /// Indicates numerical breakdown; the resulting skills are meaningless - /// and must not be treated as a converged estimate. + /// EP has broken down; the resulting skills are meaningless and must not + /// be treated as a converged estimate. Further iterations cannot recover, + /// so the loop stops rather than reporting a NaN step as convergence. #[non_exhaustive] - NonFiniteResult { - /// Where the breakdown was caught — `"History::converge"` for a sweep, - /// or a phrase naming the prediction that read an unusable skill. + NonFiniteStep { + /// Where the breakdown was caught, e.g. `"History::converge"`. context: &'static str, - /// The offending pair, at least one component of which is NaN or - /// infinite. From `converge` it is the sweep's step; from a prediction - /// it is the skill's own `(mu, sigma)`. + /// The offending step as `(|d mu|, |d sigma|)`, at least one component + /// of which is NaN or infinite. step: (f64, f64), }, + /// A prediction read a skill with no usable mean or variance. + /// + /// Split from `NonFiniteStep` (#74), which used to carry both under one + /// `step: (f64, f64)` field — a sweep step from `converge` and a skill's + /// own moments from a prediction. One field name cannot be right for both. + /// + /// Reaching this means a previous `converge` failed and its error was + /// ignored: predicting from a NaN fit produced `Ok(NaN)` on some paths and + /// a plausible-looking `Ok([0.0, 0.0])` on others. + #[non_exhaustive] + NonFiniteSkill { + /// The skill's mean, which may itself be finite while `sigma` is not. + mu: f64, + /// The skill's standard deviation. + sigma: f64, + }, + /// Every skill in the matchup is a point mass and `beta` is zero, so + /// there is no performance distribution to predict from. + /// + /// Not `InvalidParameter`: both values are individually in range, and it + /// is their combination that leaves nothing varying. Every prediction is a + /// statement about how performances vary, and in this configuration + /// nothing does — `quality` would divide by a singular contrast covariance + /// and `predict_win_probabilities` would report zeros that sum to zero. + NoPerformanceVariance, /// One batch declared two different values for the same competitor's /// configuration. /// @@ -159,9 +348,8 @@ pub enum InferenceError { /// not the user key — the batch is already flattened to indices by the /// time the conflict is detectable. competitor: usize, - /// Which piece of configuration was declared twice: `"prior"` or - /// `"drift_scale"`. - field: &'static str, + /// Which piece of configuration was declared twice. + field: CompetitorField, }, /// A prediction referenced a key the history has no skill for. /// @@ -231,14 +419,32 @@ pub enum InferenceError { /// Nodes the grid may hold. max: usize, }, - /// A joint posterior was requested where one cannot be formed exactly. - #[non_exhaustive] - JointUnavailable { - /// Why no exact joint exists here: the history has no events, it holds - /// ranked events whose EP factors are not retained past convergence, or - /// the assembled precision matrix is not positive-definite. - reason: &'static str, - }, + /// A joint posterior was requested from a history with no events. + /// + /// Split out of a single `JointUnavailable { reason: &str }` (#74): the + /// three reasons are conditions a caller branches on differently, and + /// distinguishing them used to mean matching on English prose. This one + /// means "add events". + EmptyHistory, + /// A joint posterior was requested from a history containing ranked + /// events. + /// + /// Exact only for an all-scored history: a scored likelihood is Gaussian + /// and its factor can be rebuilt exactly, while a ranked outcome's + /// truncation is approximated by EP and reconstructing those factors needs + /// the converged messages, which inference does not retain. + /// + /// [`History::predict_win_probabilities`](crate::History::predict_win_probabilities) + /// answers the comparable question on a ranked history. + JointRequiresScoredEvents, + /// The assembled precision matrix is not positive-definite. + /// + /// The usual cause is a competitor with neither a proper prior nor any + /// evidence, but an extreme prior or drift can also make the assembled + /// matrix indefinite in floating point. Unlike its two siblings this one + /// is numerical rather than structural — the same history may factorise + /// under different parameters. + NotPositiveDefinite, /// Fewer than two teams were supplied to a prediction. #[non_exhaustive] NotEnoughTeams { @@ -268,21 +474,19 @@ impl fmt::Display for InferenceError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::MismatchedShape { - kind, + shape, expected, got, } => { - write!(f, "{kind}: expected length {expected}, got {got}") + write!(f, "{shape}: expected {expected}, got {got}") } - Self::WrongOutcomeKind { - context, - expected, - got, - } => { - write!(f, "{context}: expected {expected}, got {got}") - } - Self::InvalidProbability { value } => { - write!(f, "probability must be in [0, 1]; got {value}") + Self::WrongOutcomeKind { expected, got } => { + write!( + f, + "expected {expected}, got {got}; call {} for a {} outcome", + got.constructor(), + got.adjective() + ) } Self::TieWithoutDrawProbability { teams } => { write!( @@ -303,14 +507,23 @@ impl fmt::Display for InferenceError { alpha < 1.0 if it is oscillating" ) } - Self::NonFiniteResult { context, step } => { + Self::NonFiniteStep { context, step } => { write!( f, - "{context}: inference produced a non-finite result (step = {step:?})" + "{context}: inference produced a non-finite step {step:?}; EP has \ + broken down and further iterations cannot recover" ) } - Self::InvalidParameter { name, value } => { - write!(f, "{name} is invalid: {value}") + Self::NonFiniteSkill { mu, sigma } => { + write!( + f, + "a prediction read a skill with no usable mean or variance \ + (mu = {mu}, sigma = {sigma}); the fit did not converge, and \ + `converge` reports that" + ) + } + Self::InvalidParameter { parameter, value } => { + write!(f, "{parameter} {} (got {value})", parameter.range()) } Self::ConflictingCompetitorConfig { competitor, field } => { write!( @@ -346,9 +559,25 @@ impl fmt::Display for InferenceError { one grid. Use predict_win_probabilities, which is accurate here" ) } - Self::JointUnavailable { reason } => { - write!(f, "no exact joint posterior is available: {reason}") + Self::EmptyHistory => { + f.write_str("no exact joint posterior is available: the history has no events") } + Self::JointRequiresScoredEvents => f.write_str( + "no exact joint posterior is available: the history contains ranked \ + events, whose EP factors are not retained after convergence. Use \ + predict_win_probabilities for a ranked history", + ), + Self::NotPositiveDefinite => f.write_str( + "the joint precision matrix is not positive-definite; the usual cause \ + is a competitor with neither a proper prior nor any evidence, but an \ + extreme prior or drift can also make the assembled matrix indefinite \ + in floating point", + ), + Self::NoPerformanceVariance => f.write_str( + "beta is zero and every skill in this matchup is a point mass, so \ + there is no performance distribution to predict from; give beta a \ + positive value, or a competitor a prior with positive sigma", + ), Self::NotEnoughTeams { got } => { write!(f, "prediction needs at least 2 teams, got {got}") } @@ -364,3 +593,81 @@ impl fmt::Display for InferenceError { } impl std::error::Error for InferenceError {} + +#[cfg(test)] +mod message_tests { + use super::*; + + /// Every message must name the problem *and* what to do, which is the + /// standard the good ones set and the three #74 called out did not meet. + #[test] + fn messages_are_actionable() { + let cases = [ + InferenceError::InvalidParameter { + parameter: Parameter::Alpha, + value: 0.0, + }, + InferenceError::InvalidParameter { + parameter: Parameter::DriftVariance, + value: f64::NAN, + }, + InferenceError::InvalidParameter { + parameter: Parameter::PDraw, + value: 1.5, + }, + InferenceError::MismatchedShape { + shape: Shape::OutcomeVsTeams, + expected: 3, + got: 2, + }, + InferenceError::WrongOutcomeKind { + expected: OutcomeKind::Ranked, + got: OutcomeKind::Scored, + }, + InferenceError::EmptyHistory, + InferenceError::JointRequiresScoredEvents, + InferenceError::NotPositiveDefinite, + InferenceError::NoPerformanceVariance, + InferenceError::NonFiniteSkill { + mu: f64::NAN, + sigma: f64::NAN, + }, + ]; + + for case in &cases { + let rendered = case.to_string(); + eprintln!("{rendered}"); + // `InvalidParameter` used to render `drift variance is invalid: NaN` + // — no range, no remedy, no location. Every message must at least + // be a sentence. + assert!( + rendered.len() > 30, + "message is too terse to act on: {rendered}" + ); + assert!(!rendered.contains("is invalid:"), "{rendered}"); + } + + // The three that #74 singled out now state a range or a next step. + assert!( + InferenceError::InvalidParameter { + parameter: Parameter::DriftVariance, + value: f64::NAN, + } + .to_string() + .contains("must be finite and non-negative") + ); + assert!( + InferenceError::WrongOutcomeKind { + expected: OutcomeKind::Ranked, + got: OutcomeKind::Scored, + } + .to_string() + .contains("Game::scored") + ); + assert!( + InferenceError::JointRequiresScoredEvents + .to_string() + .contains("predict_win_probabilities") + ); + } +} diff --git a/src/event_builder.rs b/src/event_builder.rs index 561a350..fc027cc 100644 --- a/src/event_builder.rs +++ b/src/event_builder.rs @@ -144,7 +144,7 @@ where if ws.len() != team.members.len() { self.error.get_or_insert(InferenceError::MismatchedShape { - kind: "weights", + shape: crate::Shape::Weights, expected: team.members.len(), got: ws.len(), }); diff --git a/src/game.rs b/src/game.rs index ebd8bf4..cf7f961 100644 --- a/src/game.rs +++ b/src/game.rs @@ -555,7 +555,7 @@ impl> Game { /// - `InvalidParameter` if `options.convergence` is out of range — an /// `alpha` of zero would leave every EP update unapplied and silently /// return the priors. - /// - `InvalidProbability` if `options.p_draw` is outside `[0.0, 1.0)`. + /// - `InvalidParameter` for a `p_draw` outside `[0.0, 1.0)`. /// - `MismatchedShape` if the outcome's rank count differs from `teams.len()`. /// - `WrongOutcomeKind` if `outcome` is not `Outcome::Ranked`. /// - `TieWithoutDrawProbability` if the outcome ties two teams while @@ -571,13 +571,14 @@ impl> Game { options.convergence.validate()?; Self::validate_teams(teams)?; if !(0.0..1.0).contains(&options.p_draw) { - return Err(crate::InferenceError::InvalidProbability { + return Err(crate::InferenceError::InvalidParameter { + parameter: crate::Parameter::PDraw, value: options.p_draw, }); } if outcome.team_count() != teams.len() { return Err(crate::InferenceError::MismatchedShape { - kind: "outcome ranks vs teams", + shape: crate::Shape::OutcomeVsTeams, expected: teams.len(), got: outcome.team_count(), }); @@ -586,9 +587,8 @@ impl> Game { let ranks = outcome .as_ranks() .ok_or(crate::InferenceError::WrongOutcomeKind { - context: "Game::ranked", - expected: "Outcome::Ranked", - got: "Outcome::Scored", + expected: crate::OutcomeKind::Ranked, + got: crate::OutcomeKind::Scored, })?; let tied = if options.p_draw == 0.0 { @@ -638,13 +638,13 @@ impl> Game { Self::validate_teams(teams)?; if options.score_sigma <= 0.0 || options.score_sigma.is_nan() { return Err(crate::InferenceError::InvalidParameter { - name: "score_sigma", + parameter: crate::Parameter::ScoreSigma, value: options.score_sigma, }); } if outcome.team_count() != teams.len() { return Err(crate::InferenceError::MismatchedShape { - kind: "outcome scores vs teams", + shape: crate::Shape::OutcomeVsTeams, expected: teams.len(), got: outcome.team_count(), }); @@ -652,9 +652,8 @@ impl> Game { let scores = outcome .as_scores() .ok_or(crate::InferenceError::WrongOutcomeKind { - context: "Game::scored", - expected: "Outcome::Scored", - got: "Outcome::Ranked", + expected: crate::OutcomeKind::Scored, + got: crate::OutcomeKind::Ranked, })? .to_vec(); // A non-finite score poisons the chain rather than failing it. Ranks @@ -662,7 +661,7 @@ impl> Game { for value in &scores { if !value.is_finite() { return Err(crate::InferenceError::InvalidParameter { - name: "score", + parameter: crate::Parameter::Score, value: *value, }); } @@ -1368,7 +1367,7 @@ mod tests { assert!(matches!( err, crate::InferenceError::InvalidParameter { - name: "score_sigma", + parameter: crate::Parameter::ScoreSigma, .. } )); diff --git a/src/gaussian.rs b/src/gaussian.rs index dd52295..c987fdb 100644 --- a/src/gaussian.rs +++ b/src/gaussian.rs @@ -22,7 +22,7 @@ impl Gaussian { /// /// Panics if `sigma` is negative. NaN is deliberately allowed through: a /// broken fit produces one, and `converge` reports that as - /// `NonFiniteResult` rather than panicking mid-inference. + /// `NonFiniteStep` rather than panicking mid-inference. /// /// A negative sigma used to be accepted and returned results **bit /// identical** to its absolute value, because sigma only ever enters as @@ -46,7 +46,7 @@ impl Gaussian { pub const fn from_ms(mu: f64, sigma: f64) -> Self { // NaN is admitted on purpose. A broken fit legitimately produces a NaN // sigma — `sqrt` of a negative truncated variance — and the design is - // to propagate that to `converge`'s `NonFiniteResult` guard, not to + // to propagate that to `converge`'s `NonFiniteStep` guard, not to // panic inside inference. Rejecting it here turned that reporting path // into a crash, which two tests caught immediately. assert!( diff --git a/src/history.rs b/src/history.rs index b2010a8..33e305d 100644 --- a/src/history.rs +++ b/src/history.rs @@ -72,7 +72,7 @@ impl, O: Observer, K: Eq + Hash + Clone, R: RatingRule Self { assert!(mu.is_finite(), "mu must be finite (got {mu})"); @@ -855,14 +855,14 @@ impl, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule(&self, teams: &[&[&Q]]) -> Result where @@ -1481,7 +1477,7 @@ impl, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule Result, InferenceError> { if self.time_slices.is_empty() { - return Err(InferenceError::JointUnavailable { - reason: "the history has no events", - }); + return Err(InferenceError::EmptyHistory); } if !self.time_slices.iter().all(TimeSlice::all_scored) { - return Err(InferenceError::JointUnavailable { - reason: "the history contains ranked events, whose EP factors are \ - not retained after convergence", - }); + return Err(InferenceError::JointRequiresScoredEvents); } let TimeExpanded { @@ -1728,14 +1720,8 @@ impl, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule(&self, teams: &[&[&Q]]) -> Result where K: Borrow, @@ -1780,7 +1766,7 @@ impl, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule(&self, teams: &[&[&Q]]) -> Result where @@ -1908,9 +1894,9 @@ impl, O: Observer, K: Eq + Hash + Clone, R: RatingRule(&self, teams: &[&[&Q]]) -> Result, InferenceError> where @@ -1963,9 +1949,9 @@ impl, O: Observer, K: Eq + Hash + Clone, R: RatingRule(&self, teams: &[&[&Q]]) -> Result where @@ -2013,9 +1999,9 @@ impl, O: Observer, K: Eq + Hash + Clone, R: RatingRule(&self, teams: &[&[&Q]], ranks: &[u32]) -> Result where @@ -2024,7 +2010,7 @@ impl, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule "rank", - EventKind::Scored { .. } => "score", + let parameter = match kind { + EventKind::Ranked => crate::Parameter::Rank, + EventKind::Scored { .. } => crate::Parameter::Score, }; for value in event_results { if !value.is_finite() { return Err(InferenceError::InvalidParameter { - name, + parameter, value: *value, }); } @@ -2289,7 +2275,7 @@ impl, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, K: Eq + Hash + Clone, R: RatingRule, O: Observer, R: RatingRule Result { if winner >= n { return Err(crate::InferenceError::InvalidParameter { - name: "winner", + parameter: crate::Parameter::WinnerIndex, value: f64::from(winner), }); } diff --git a/tests/competitor_config.rs b/tests/competitor_config.rs index ba0cc0f..403451a 100644 --- a/tests/competitor_config.rs +++ b/tests/competitor_config.rs @@ -184,7 +184,10 @@ fn a_batch_declaring_two_different_priors_is_rejected() { assert!( matches!( err, - InferenceError::ConflictingCompetitorConfig { field: "prior", .. } + InferenceError::ConflictingCompetitorConfig { + field: trueskill_tt::CompetitorField::Prior, + .. + } ), "got {err:?}" ); diff --git a/tests/degenerate_inputs.rs b/tests/degenerate_inputs.rs index e2be13e..de843e9 100644 --- a/tests/degenerate_inputs.rs +++ b/tests/degenerate_inputs.rs @@ -157,7 +157,7 @@ fn event_builder_rejects_a_weights_length_mismatch() { matches!( err, InferenceError::MismatchedShape { - kind: "weights", + shape: trueskill_tt::Shape::Weights, expected: 1, got: 2, .. @@ -228,7 +228,7 @@ fn scored_event_rejects_non_positive_sigma() { assert!(matches!( err, InferenceError::InvalidParameter { - name: "score_sigma", + parameter: trueskill_tt::Parameter::ScoreSigma, .. } )); diff --git a/tests/drift_scale.rs b/tests/drift_scale.rs index 64aaf0e..70d0bcf 100644 --- a/tests/drift_scale.rs +++ b/tests/drift_scale.rs @@ -279,7 +279,7 @@ fn reject(scale: f64) -> InferenceError { fn negative_scale_is_rejected() { assert!(matches!( reject(-1.0), - InferenceError::InvalidParameter { name: "drift_scale", value, .. } + InferenceError::InvalidParameter { parameter: trueskill_tt::Parameter::DriftScale, value, .. } if value == -1.0 )); } @@ -291,7 +291,7 @@ fn non_finite_scale_is_rejected() { matches!( reject(scale), InferenceError::InvalidParameter { - name: "drift_scale", + parameter: trueskill_tt::Parameter::DriftScale, .. } ), @@ -488,7 +488,7 @@ fn a_batch_that_contradicts_itself_is_rejected() { matches!( err, InferenceError::ConflictingCompetitorConfig { - field: "drift_scale", + field: trueskill_tt::CompetitorField::DriftScale, .. } ), diff --git a/tests/event_builder_members.rs b/tests/event_builder_members.rs index 0aceb47..ad45e1a 100644 --- a/tests/event_builder_members.rs +++ b/tests/event_builder_members.rs @@ -136,7 +136,7 @@ fn weights_still_guards_a_members_team() { matches!( err, InferenceError::MismatchedShape { - kind: "weights", + shape: trueskill_tt::Shape::Weights, expected: 2, got: 1, .. @@ -164,7 +164,7 @@ fn an_invalid_drift_scale_surfaces_from_commit() { matches!( err, InferenceError::InvalidParameter { - name: "drift_scale", + parameter: trueskill_tt::Parameter::DriftScale, .. } ), diff --git a/tests/game.rs b/tests/game.rs index 35905b5..d2e5ec9 100644 --- a/tests/game.rs +++ b/tests/game.rs @@ -57,7 +57,7 @@ fn game_ranked_rejects_bad_p_draw() { }, ) .unwrap_err(); - assert!(matches!(err, InferenceError::InvalidProbability { .. })); + assert!(matches!(err, InferenceError::InvalidParameter { .. })); } #[test] @@ -227,7 +227,13 @@ mod malformed_games { ) .unwrap_err(); assert!( - matches!(err, InferenceError::InvalidParameter { name: "score", .. }), + matches!( + err, + InferenceError::InvalidParameter { + parameter: trueskill_tt::Parameter::Score, + .. + } + ), "{bad}: {err:?}" ); } diff --git a/tests/ingestion_shape.rs b/tests/ingestion_shape.rs index 7d27ff9..6f5eb65 100644 --- a/tests/ingestion_shape.rs +++ b/tests/ingestion_shape.rs @@ -112,7 +112,13 @@ fn a_non_finite_score_is_rejected_at_ingestion() { }]) .unwrap_err(); assert!( - matches!(err, InferenceError::InvalidParameter { name: "score", .. }), + matches!( + err, + InferenceError::InvalidParameter { + parameter: trueskill_tt::Parameter::Score, + .. + } + ), "{bad}: {err:?}" ); assert!(h.current_skill(&"a").is_none(), "{bad} was recorded anyway"); @@ -136,7 +142,13 @@ fn a_non_finite_weight_is_rejected_at_ingestion() { .commit() .unwrap_err(); assert!( - matches!(err, InferenceError::InvalidParameter { name: "weight", .. }), + matches!( + err, + InferenceError::InvalidParameter { + parameter: trueskill_tt::Parameter::Weight, + .. + } + ), "{bad}: {err:?}" ); assert!(h.current_skill(&"a").is_none(), "{bad} reached the history"); diff --git a/tests/joint_handle.rs b/tests/joint_handle.rs index a913439..1daaef8 100644 --- a/tests/joint_handle.rs +++ b/tests/joint_handle.rs @@ -231,16 +231,20 @@ fn a_ranked_history_has_no_exact_joint() { let _ = h.converge().unwrap(); assert!(matches!( h.joint().unwrap_err(), - InferenceError::JointUnavailable { .. } + InferenceError::JointRequiresScoredEvents )); } +/// Distinguishable from the ranked case, which is the point of splitting +/// `JointUnavailable { reason: &str }` into three variants (#74): "add events" +/// and "use predict_win_probabilities" are different instructions, and telling +/// them apart used to mean matching on English prose. #[test] fn an_empty_history_has_no_joint() { let h = history(UnknownKeys::Reject); assert!(matches!( h.joint().unwrap_err(), - InferenceError::JointUnavailable { .. } + InferenceError::EmptyHistory )); } diff --git a/tests/non_finite_results.rs b/tests/non_finite_results.rs index 978b7b6..dad9855 100644 --- a/tests/non_finite_results.rs +++ b/tests/non_finite_results.rs @@ -56,7 +56,7 @@ fn overflow_during_inference_is_reported_not_hidden() { for (name, sigma, beta, score_sigma, scores) in cases { match scored_fit(sigma, beta, score_sigma, scores) { - Err(InferenceError::NonFiniteResult { context, step, .. }) => { + Err(InferenceError::NonFiniteStep { context, step, .. }) => { assert_eq!(context, "History::converge", "{name}"); assert!( !step.0.is_finite() || !step.1.is_finite(), @@ -86,7 +86,7 @@ fn a_broken_fit_is_never_reported_as_converged() { let err = h.converge().unwrap_err(); assert!( - matches!(err, InferenceError::NonFiniteResult { .. }), + matches!(err, InferenceError::NonFiniteStep { .. }), "a breakdown must not be reported as convergence: {err:?}" ); @@ -104,7 +104,7 @@ fn a_broken_fit_is_never_reported_as_converged() { .unwrap(); assert!(matches!( h2.converge_partial().unwrap_err(), - InferenceError::NonFiniteResult { .. } + InferenceError::NonFiniteStep { .. } )); } @@ -161,7 +161,7 @@ fn a_nan_competitor_is_not_masked_by_a_healthy_one() { .converge() .expect_err("a NaN fit must never be reported as converged"); assert!( - matches!(err, InferenceError::NonFiniteResult { .. }), + matches!(err, InferenceError::NonFiniteStep { .. }), "{err:?}" ); } diff --git a/tests/prediction_guards.rs b/tests/prediction_guards.rs index 4cc106c..99cfed8 100644 --- a/tests/prediction_guards.rs +++ b/tests/prediction_guards.rs @@ -49,7 +49,7 @@ fn nan_poisoned() -> H { ); let err = h.converge().expect_err("this fixture must not converge"); assert!( - matches!(err, InferenceError::NonFiniteResult { .. }), + matches!(err, InferenceError::NonFiniteStep { .. }), "{err:?}" ); h @@ -101,7 +101,7 @@ fn a_nan_poisoned_fit_is_refused_by_every_prediction_path() { all_predictions!(h, |name: &str, r: Result<(), InferenceError>| { match r { - Err(InferenceError::NonFiniteResult { .. }) => {} + Err(InferenceError::NonFiniteSkill { .. }) => {} other => panic!("{name} answered from a NaN fit: {other:?}"), } }); @@ -121,7 +121,7 @@ fn degenerate_performances_are_refused_rather_than_answered_wrongly() { // and every skill is a point mass. all_predictions!(h, |name: &str, r: Result<(), InferenceError>| { match r { - Err(InferenceError::InvalidParameter { .. }) => {} + Err(InferenceError::NoPerformanceVariance) => {} other => panic!("{name} predicted from a degenerate fit: {other:?}"), } }); diff --git a/tests/registration.rs b/tests/registration.rs index f165622..1467184 100644 --- a/tests/registration.rs +++ b/tests/registration.rs @@ -172,7 +172,13 @@ fn a_weight_on_a_registration_is_rejected() { .register(Member::new("layout").with_weight(0.5)) .unwrap_err(); assert!( - matches!(err, InferenceError::InvalidParameter { name: "weight", .. }), + matches!( + err, + InferenceError::InvalidParameter { + parameter: trueskill_tt::Parameter::Weight, + .. + } + ), "{err:?}" ); } @@ -188,7 +194,7 @@ fn an_invalid_drift_scale_on_a_registration_is_rejected() { matches!( err, InferenceError::InvalidParameter { - name: "drift_scale", + parameter: trueskill_tt::Parameter::DriftScale, .. } ), @@ -280,7 +286,7 @@ mod conflicting_configuration { matches!( err, InferenceError::ConflictingCompetitorConfig { - field: "drift_scale", + field: trueskill_tt::CompetitorField::DriftScale, .. } ), @@ -297,7 +303,7 @@ mod conflicting_configuration { matches!( err, InferenceError::ConflictingCompetitorConfig { - field: "drift_scale", + field: trueskill_tt::CompetitorField::DriftScale, .. } ), diff --git a/tests/validation.rs b/tests/validation.rs index 49d1ce2..9996429 100644 --- a/tests/validation.rs +++ b/tests/validation.rs @@ -49,7 +49,13 @@ fn ranked_rejects_a_zero_damping_factor() { ) .expect_err("alpha = 0 must be rejected"); assert!( - matches!(err, InferenceError::InvalidParameter { name: "alpha", .. }), + matches!( + err, + InferenceError::InvalidParameter { + parameter: trueskill_tt::Parameter::Alpha, + .. + } + ), "got {err:?}" ); } @@ -65,7 +71,13 @@ fn ranked_rejects_an_out_of_range_damping_factor() { ) .expect_err("alpha out of (0, 1] must be rejected"); assert!( - matches!(err, InferenceError::InvalidParameter { name: "alpha", .. }), + matches!( + err, + InferenceError::InvalidParameter { + parameter: trueskill_tt::Parameter::Alpha, + .. + } + ), "alpha={alpha}: got {err:?}" ); } @@ -81,7 +93,13 @@ fn scored_rejects_a_bad_damping_factor() { ) .expect_err("alpha = 0 must be rejected"); assert!( - matches!(err, InferenceError::InvalidParameter { name: "alpha", .. }), + matches!( + err, + InferenceError::InvalidParameter { + parameter: trueskill_tt::Parameter::Alpha, + .. + } + ), "got {err:?}" ); } @@ -360,7 +378,7 @@ mod constructor_parameters { matches!( err, InferenceError::InvalidParameter { - name: "drift variance", + parameter: trueskill_tt::Parameter::DriftVariance, .. } ),