diff --git a/src/history.rs b/src/history.rs index e904a43..40befe9 100644 --- a/src/history.rs +++ b/src/history.rs @@ -39,17 +39,55 @@ pub struct HistoryBuilder< } impl, O: Observer, K: Eq + Hash + Clone> HistoryBuilder { + /// Prior mean skill. + /// + /// # Panics + /// + /// Panics if `mu` is not finite. A non-finite prior mean poisons every + /// posterior derived from it: `converge` reports `NonFiniteResult`, but a + /// caller who reads `current_skill` first is handed `tau: NaN`. pub fn mu(mut self, mu: f64) -> Self { + assert!(mu.is_finite(), "mu must be finite (got {mu})"); self.mu = mu; self } + /// Prior standard deviation. + /// + /// # Panics + /// + /// Panics unless `sigma` is finite and strictly positive. + /// + /// Zero and infinity both give a prior precision that is not a number, and + /// the whole fit comes back NaN. A *negative* sigma is the quieter half: + /// it is only ever squared, so `-8.33` produces bit-identical results to + /// `8.33` — a sign the caller cannot have meant, silently ignored. pub fn sigma(mut self, sigma: f64) -> Self { + assert!( + sigma.is_finite() && sigma > 0.0, + "sigma must be finite and positive (got {sigma})" + ); self.sigma = sigma; self } + /// Per-event performance noise. + /// + /// # Panics + /// + /// Panics unless `beta` is finite and non-negative. + /// + /// Zero is allowed and meaningful — performance is then exactly skill, and + /// the fit differs measurably from a positive `beta` rather than + /// degenerating. Negative is rejected for the same reason as a negative + /// `sigma` or `Member::with_drift_scale`: `beta` enters only as `beta^2`, + /// so a negative value behaves as its absolute value and the sign is lost + /// without comment. pub fn beta(mut self, beta: f64) -> Self { + assert!( + beta.is_finite() && beta >= 0.0, + "beta must be finite and non-negative (got {beta})" + ); self.beta = beta; self } diff --git a/tests/validation.rs b/tests/validation.rs index 2bb9cea..9ec4686 100644 --- a/tests/validation.rs +++ b/tests/validation.rs @@ -184,3 +184,74 @@ fn ingestion_rejects_weights_that_do_not_match_their_team() { "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); + assert!(zero.pi().is_finite() && zero.pi() > 0.0); + assert!( + (zero.pi() - positive.pi()).abs() > 1e-6, + "zero beta must not merely be ignored: {zero:?} vs {positive:?}" + ); + } +}