Merge branch 'feat/convergence-strictness'

Make a short fit an error, raise the default iteration cap, validate the
remaining HistoryBuilder parameters, add History::register and
History::rating, reject competitor config conflicts across batches, and
document what the joint's cost scales in.

Closes #50

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
This commit is contained in:
2026-09-08 21:00:39 +02:00
co-authored by Claude Opus 5
8 changed files with 996 additions and 27 deletions
+10 -3
View File
@@ -62,10 +62,17 @@ impl Default for ConvergenceOptions {
}
/// Post-hoc summary of a `History::converge` call.
///
/// From [`History::converge`](crate::History::converge) this always describes a
/// converged fit — stopping at `max_iter` is
/// [`InferenceError::NotConverged`](crate::InferenceError::NotConverged) there.
/// From [`History::converge_partial`](crate::History::converge_partial) it may
/// not be, and `converged` is what says so.
#[derive(Clone, Debug)]
#[must_use = "a ConvergenceReport carries `converged`, and a fit that stopped \
at `max_iter` is wrong by a little rather than loudly broken — \
check it, or bind it to `_` to say you have decided not to"]
#[must_use = "from `converge_partial` this may describe a fit that stopped at \
`max_iter`, which is wrong by a little rather than loudly \
broken — check `converged`, or bind it to `_` to say you have \
decided not to"]
pub struct ConvergenceReport {
pub iterations: usize,
pub final_step: (f64, f64),
+49
View File
@@ -64,6 +64,24 @@ pub enum InferenceError {
/// result has no representable likelihood. Configure a positive `p_draw`
/// (via `HistoryBuilder::p_draw` or `GameOptions::p_draw`) to admit ties.
TieWithoutDrawProbability { teams: (usize, usize) },
/// The convergence sweep hit `max_iter` with the step still above
/// `epsilon`.
///
/// A fit that stops short is wrong by a little, which is the worst
/// available failure: every rating is finite, the ordering looks sensible,
/// and nothing in the numbers says they were still moving. Reported rather
/// than returned as a flag on an `Ok`, because a flag has to be checked
/// and `let _ = h.converge()` is the natural way not to.
///
/// Either the history needs more iterations — raise `max_iter` — or it is
/// oscillating rather than converging, in which case `alpha < 1.0` damps
/// the within-game EP loop. [`History::converge_partial`](crate::History::converge_partial)
/// returns the short fit instead when that is genuinely what is wanted.
NotConverged {
iterations: usize,
final_step: (f64, f64),
epsilon: f64,
},
/// Inference produced a non-finite value (NaN or infinity).
///
/// Indicates numerical breakdown; the resulting skills are meaningless
@@ -100,6 +118,17 @@ pub enum InferenceError {
member: usize,
key: String,
},
/// `History::register` was called for a competitor that already exists.
///
/// Registration states a competitor's configuration before anything has
/// been observed about them, so a competitor that already exists has
/// already been configured — by an earlier `register`, or by an event that
/// created them. Silently overwriting would reintroduce exactly the
/// order-dependence registration exists to remove.
///
/// To change an existing competitor's configuration, supply it on an event
/// through `Member`; that refits the whole history.
AlreadyRegistered { key: String },
/// A prediction was given a team with no members.
EmptyTeam { team: usize },
/// A joint posterior was requested where one cannot be formed exactly.
@@ -144,6 +173,18 @@ impl fmt::Display for InferenceError {
teams.0, teams.1
)
}
Self::NotConverged {
iterations,
final_step,
epsilon,
} => {
write!(
f,
"did not converge in {iterations} iterations: final step {final_step:?} \
is still above epsilon {epsilon}; raise max_iter, or damp with \
alpha < 1.0 if it is oscillating"
)
}
Self::NonFiniteResult { context, step } => {
write!(
f,
@@ -167,6 +208,14 @@ impl fmt::Display for InferenceError {
with `lookup` or `current_skill` if that is not guaranteed)"
)
}
Self::AlreadyRegistered { key } => {
write!(
f,
"competitor {key} is already registered; registration states \
configuration before anything is observed, so re-registering \
would silently overwrite it"
)
}
Self::EmptyTeam { team } => {
write!(f, "team {team} has no members")
}
+294 -10
View File
@@ -6,6 +6,7 @@ use crate::{
convergence::{ConvergenceOptions, ConvergenceReport},
drift::{ConstantDrift, Drift},
error::InferenceError,
event::Member,
gaussian::Gaussian,
key_table::KeyTable,
observer::{NullObserver, Observer},
@@ -39,17 +40,55 @@ pub struct HistoryBuilder<
}
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<T, D, O, K> {
/// 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
}
@@ -169,6 +208,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
convergence: self.convergence,
observer: self.observer,
unknown_keys: self.unknown_keys,
declared: HashMap::new(),
}
}
}
@@ -276,6 +316,12 @@ pub struct History<
convergence: ConvergenceOptions,
observer: O,
unknown_keys: crate::UnknownKeys,
/// Competitor configuration explicitly declared so far, by whichever route.
///
/// Kept separate from the applied `Rating` because a `Rating` cannot say
/// whether a value was *chosen* or inherited from the history defaults,
/// and that is exactly the distinction a conflict check needs.
declared: HashMap<Index, CompetitorConfig>,
}
impl Default for History<i64, ConstantDrift, NullObserver, &'static str> {
@@ -455,6 +501,111 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
}
/// Skill estimate at the latest time slice the competitor appears in.
/// Configure a competitor before anything has been observed about them.
///
/// The configuration a competitor needs is often a property of the domain
/// rather than of any one event — "every layout is static", "this bot sits
/// at a known strength". Stating it per-event means every ingestion path
/// has to remember it, and the fluent and two-argument paths could not
/// state it at all.
///
/// ```
/// # use trueskill_tt::{History, Member};
/// let mut h = History::builder().build();
/// h.register(Member::new("layout_7").with_drift_scale(0.0))?;
///
/// // Reaches a competitor first seen through any route, including the
/// // two-argument one, which cannot carry configuration itself.
/// h.record_winner(&"player", &"layout_7", 1)?;
/// assert_eq!(h.rating(&"layout_7").unwrap().drift_scale(), 0.0);
/// # Ok::<(), trueskill_tt::InferenceError>(())
/// ```
///
/// The competitor exists from this point on, with no appearances, so
/// [`History::rating`] can read back what was actually stored — the
/// diagnostic that was previously missing entirely.
///
/// `weight` is per-event and has no meaning here, so a `Member` carrying a
/// non-default one is rejected rather than silently ignored.
///
/// # Errors
///
/// `AlreadyRegistered` if the competitor already exists, whether from an
/// earlier `register` or from an event. `InvalidParameter` for a `weight`
/// other than 1.0, or a `drift_scale` that is negative or non-finite.
pub fn register(&mut self, member: Member<K>) -> Result<(), InferenceError>
where
K: std::fmt::Debug,
{
if member.weight != 1.0 {
return Err(InferenceError::InvalidParameter {
name: "weight",
value: member.weight,
});
}
if let Some(scale) = member.drift_scale {
if !scale.is_finite() || scale < 0.0 {
return Err(InferenceError::InvalidParameter {
name: "drift_scale",
value: scale,
});
}
}
let key = format!("{:?}", member.key);
let idx = self.keys.get_or_create(&member.key);
if self.agents.contains(idx) {
return Err(InferenceError::AlreadyRegistered { key });
}
let mut rating = Rating::new(
Gaussian::from_ms(self.mu, self.sigma),
self.beta,
self.drift,
);
if let Some(prior) = member.prior {
rating.prior = prior;
}
if let Some(scale) = member.drift_scale {
rating.drift_scale = scale;
}
self.declared.insert(
idx,
CompetitorConfig {
prior: member.prior,
drift_scale: member.drift_scale,
},
);
self.agents.insert(
idx,
Competitor {
rating,
message: None,
last_time: None,
},
);
Ok(())
}
/// The configuration in force for a competitor, or `None` if the history
/// has never seen them.
///
/// Reads back what was actually stored, which is what makes a
/// configuration mistake detectable from outside the crate. Every other
/// accessor returns what inference *inferred*; this returns what it was
/// told.
#[must_use]
pub fn rating<Q>(&self, key: &Q) -> Option<Rating<T, D>>
where
K: std::borrow::Borrow<Q>,
Q: std::hash::Hash + Eq + ?Sized,
{
let idx = self.keys.get(key)?;
self.agents.contains(idx).then(|| self.agents[idx].rating)
}
pub fn current_skill<Q>(&self, key: &Q) -> Option<Gaussian>
where
K: std::borrow::Borrow<Q>,
@@ -958,6 +1109,14 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
/// [`Joint`] with [`History::joint`] instead — the answers are identical,
/// and only the first one pays.
///
/// # Cost
///
/// A dense solve over the history's *appearances*, not its competitors. A
/// drift-free competitor collapses to a single variable however long the
/// history, so the same events can differ enormously in cost depending on
/// the drift configuration — see [`Joint`], which also amortises this
/// across many questions.
///
/// # Limitations
///
/// Exact only for a history whose events are all scored, because a scored
@@ -1390,17 +1549,62 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
))
}
/// Run the full forward+backward convergence loop and return a summary.
/// Run the full forward+backward convergence loop to a fixed point.
///
/// Failing to reach `epsilon` within `max_iter` is not an error: the
/// returned report carries `converged: false` and the final step.
/// # Stopping short is an error
///
/// Hitting `max_iter` without reaching `epsilon` returns `NotConverged`.
///
/// 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
/// rating is finite, the ordering looks sensible, and nothing about the
/// output says the numbers were still moving. Detection was opt-in, and
/// `let _ = h.converge()` silently opted out — which is how a real defect
/// hid in this crate's own test suite.
///
/// The default `max_iter` is [`ITERATIONS`](crate::ITERATIONS), which is
/// set high enough that reaching it means something is genuinely wrong
/// rather than that the history is merely large. Raising the cap costs
/// nothing when it is not needed, because the loop exits at `epsilon`.
///
/// Use [`History::converge_partial`] when a capped, unconverged fit is
/// what you actually want.
///
/// # Errors
///
/// `NotConverged` if the sweep hits `max_iter` with the step still above
/// `epsilon`.
///
/// `NonFiniteResult` if a sweep produces a NaN or infinite step. EP has
/// broken down at that point and further iterations cannot recover, so the
/// loop stops rather than reporting a NaN step as convergence.
pub fn converge(&mut self) -> Result<ConvergenceReport, InferenceError> {
let report = self.converge_partial()?;
if report.converged {
Ok(report)
} else {
Err(InferenceError::NotConverged {
iterations: report.iterations,
final_step: report.final_step,
epsilon: self.convergence.epsilon,
})
}
}
/// As [`History::converge`], but a fit that stops at `max_iter` is
/// returned rather than reported as an error.
///
/// The report's `converged` flag says which happened. Use this when a
/// deliberately capped sweep is the point — a cheap approximate fit, or a
/// test that pins what a fixed number of iterations produces. Prefer
/// `converge` everywhere else: an unconverged fit that nobody checks is
/// indistinguishable from a converged one.
///
/// # Errors
///
/// `NonFiniteResult` if a sweep produces a NaN or infinite step.
pub fn converge_partial(&mut self) -> Result<ConvergenceReport, InferenceError> {
use std::time::Instant;
use smallvec::SmallVec;
@@ -1564,6 +1768,47 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
}
}
// Cross-batch conflict. The in-batch check upstream rejects one batch
// that sets a field twice; `priors` is rebuilt per call, so without
// this a *second* batch could quietly overwrite what a first one
// declared, last-write-wins.
//
// That asymmetry cut against the invariant `tests/ingestion_equivalence.rs`
// exists to protect: the same contradictory events errored when
// batched and succeeded, order-dependently, when fed one at a time.
// Checked before anything mutates, so a rejected batch leaves the
// history untouched.
for (agent, batch) in &priors {
let held = self.declared.get(agent).copied().unwrap_or_default();
if let (Some(existing), Some(new)) = (held.prior, batch.prior) {
if existing != new {
return Err(InferenceError::ConflictingCompetitorConfig {
competitor: agent.get(),
field: "prior",
});
}
}
if let (Some(existing), Some(new)) = (held.drift_scale, batch.drift_scale) {
if existing != new {
return Err(InferenceError::ConflictingCompetitorConfig {
competitor: agent.get(),
field: "drift_scale",
});
}
}
}
for (agent, batch) in &priors {
let entry = self.declared.entry(*agent).or_default();
if batch.prior.is_some() {
entry.prior = batch.prior;
}
if batch.drift_scale.is_some() {
entry.drift_scale = batch.drift_scale;
}
}
competitor::clean(self.agents.values_mut(), true);
let mut this_agent = Vec::with_capacity(1024);
@@ -1575,7 +1820,9 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
this_agent.push(*agent);
let config = priors.get(agent).copied().unwrap_or_default();
// From `declared` rather than `priors`: a competitor configured by
// `register` before any event has nothing in this batch's map.
let config = self.declared.get(agent).copied().unwrap_or_default();
if self.agents.contains(*agent) {
// Seeding a competitor the history already knows. This used to
@@ -1966,6 +2213,34 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
/// added while it is alive. That is what makes it correct without any
/// invalidation logic: there is no window in which the factorisation could
/// describe a fit that no longer exists.
///
/// # What the cost actually scales in
///
/// Not competitors, and not slices times competitors. One variable per
/// *appearance* — a competitor per slice they appear in — minus every
/// consecutive pair with no drift between them, which collapse to a single
/// latent variable.
///
/// That last clause dominates, and it is not obvious. A competitor whose drift
/// is zero contributes **one** variable however long the history: whole-history
/// `gamma = 0`, or `drift_scale = 0` on that competitor. So two fits over the
/// same events and the same slices can differ in problem size by roughly the
/// slice count, and in factorisation time by its cube. Measured by a consumer
/// on a ~2,000-node model over 76 slices:
///
/// ```text
/// career fit (gamma = 0) 787 ms per solve
/// drifting fit (gamma = 0.15) 6214 ms per solve
/// ```
///
/// Choosing between a drifting and a drift-free configuration is therefore also
/// choosing an 8x difference in query cost. [`Joint::variables`] reports the
/// number that decides it, and can be read before committing to a batch of
/// queries.
///
/// Slices a competitor sits out cost nothing: an absence is not an appearance,
/// so a competitor seen in the first and last of a hundred slices contributes
/// two variables, not a hundred.
pub struct Joint<'h, T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> {
history: &'h History<T, D, O, K>,
cholesky: crate::joint::Cholesky,
@@ -1993,9 +2268,14 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> Joint<'_, T, D,
/// Number of variables in the joint: the history's appearances, after
/// collapsing consecutive pairs a competitor does not drift between.
///
/// This is what the cost scales in, and it is not the competitor count — a
/// competitor contributes one variable per slice it appears in. Worth
/// checking before asking for a joint over a long history.
/// This is what the cost scales in — `O(n^3)` to factorise, `O(n^2)` per
/// query — and it is neither the competitor count nor slices times
/// competitors. A drift-free competitor contributes one variable however
/// many slices they appear in; see the type docs for how large that
/// difference gets.
///
/// Worth reading before committing to a batch of queries: it is the one
/// number that says whether a joint over this history is affordable.
#[must_use]
pub fn variables(&self) -> usize {
self.width
@@ -2799,13 +3079,15 @@ mod tests {
epsilon = 1e-6
);
// run exactly 11 iterations (old test used convergence(11, ...))
// Run exactly 11 iterations. `converge_partial` rather than
// `converge`: stopping at the cap is the point here, and `converge`
// now reports that as `NotConverged`.
h.convergence = ConvergenceOptions {
max_iter: 11,
epsilon: EPSILON,
alpha: 1.0,
};
let _ = h.converge().unwrap();
let _ = h.converge_partial().unwrap();
let loocv_approx_2 = h.log_evidence_internal(false, &[]).exp().sqrt();
@@ -3172,7 +3454,9 @@ mod tests {
})
.build();
events_for(&mut h_capped);
let _ = h_capped.converge().unwrap();
// A one-iteration cap is deliberate here, so the short fit is the
// result rather than an error.
let _ = h_capped.converge_partial().unwrap();
let mut h_full: History<i64, _, _, &'static str> = History::builder().build();
events_for(&mut h_full);
+35 -14
View File
@@ -158,22 +158,43 @@ pub const P_DRAW: f64 = 0.0;
pub const EPSILON: f64 = 1e-6;
/// Default cap on convergence sweeps.
///
/// **This is a floor, not a recommendation.** It is adequate for small
/// histories and is quickly outgrown: a history of 400 events over 100
/// competitors already stops here with a final step of ~7e-3 against the 1e-6
/// default tolerance — four orders of magnitude short — and a dense joint model
/// of ~2,000 nodes over ~3,300 events has been measured needing 76 to 161.
/// **A runaway guard, not a budget.** The sweep exits as soon as the step falls
/// below `epsilon`, so the cap is never reached by a history that converges and
/// raising it costs nothing. Measured on a history that needs four sweeps:
///
/// Overrunning it is not an error, and deliberately so: `converge` returns a
/// [`ConvergenceReport`] whose `converged` flag says what happened. But a fit
/// that stopped short is *wrong by a little*, which is the worst available
/// failure — every rating is finite and ordered sensibly, and nothing in the
/// numbers themselves says they were still moving. Read the report; the type is
/// `#[must_use]` for that reason.
/// ```text
/// max_iter 30: 4 iterations, 129.9 us
/// max_iter 100_000: 4 iterations, 131.9 us
/// ```
///
/// Raise it via [`ConvergenceOptions`]. Convergence cost is roughly linear in
/// the cap, and for anything but a toy the extra sweeps are milliseconds.
pub const ITERATIONS: usize = 30;
/// This was `30` until it was measured, and 30 truncated ordinary healthy
/// histories: 160 events over 100 competitors already needs 42. Because a short
/// fit is finite and sensibly ordered, that was invisible.
///
/// # Why it is not scaled to the history
///
/// The obvious improvement — pick the cap from the node or event count — does
/// not work, because iteration count is driven by how *loopy* the graph is
/// rather than how big it is. At a fixed 320 events over 40 slices, varying
/// only the number of competitors sharing them:
///
/// ```text
/// competitors appearances each iterations
/// 3 213 2_789
/// 10 64 1_068
/// 50 12.8 206
/// 100 6.4 90
/// 400 1.6 2
/// ```
///
/// Three orders of magnitude apart on identical event and slice counts. Any
/// formula in those two numbers would be badly wrong on some real shape, so the
/// cap is a single value set high enough that reaching it means the fit is
/// oscillating rather than merely large.
///
/// Reaching it is [`InferenceError::NotConverged`]. See
/// [`History::converge`](crate::History::converge).
pub const ITERATIONS: usize = 10_000;
/// Largest team count `History::predict_outcome` will enumerate.
///
+152
View File
@@ -0,0 +1,152 @@
//! 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(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_with_key()
.mu(0.0)
.sigma(6.0)
.beta(1.0)
.score_sigma(2.0)
.drift(ConstantDrift(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);
}
+47
View File
@@ -141,6 +141,53 @@ fn variables_counts_appearances_not_competitors() {
assert_eq!(joint.variables(), 12);
}
/// How much the collapse is worth, which is the part a caller has to plan
/// around: a drift-free competitor contributes **one** variable however long
/// the history, so the same events at `gamma = 0` and `gamma > 0` differ by
/// roughly the slice count in problem size — and by its cube in solve time.
///
/// Reported by a consumer as an 8x difference in solve time on a ~2,000-node,
/// 76-slice model (787 ms career against 6,214 ms drifting). This pins the
/// mechanism behind that so a change to the collapse rule cannot quietly
/// remove it.
#[test]
fn drift_free_competitors_shrink_the_joint_by_the_slice_count() {
fn variables(gamma: f64) -> usize {
let mut h = History::builder()
.mu(0.0)
.sigma(6.0)
.beta(1.0)
.score_sigma(2.0)
.drift(ConstantDrift(gamma))
.convergence(ConvergenceOptions {
max_iter: 20_000,
epsilon: 1e-13,
alpha: 1.0,
})
.build();
h.add_events(
(1..=10)
.map(|t| duel("a", "b", t, 5.0, 2.0))
.collect::<Vec<_>>(),
)
.unwrap();
let _ = h.converge().unwrap();
h.joint().unwrap().variables()
}
let drifting = variables(0.5);
let career = variables(0.0);
// Two competitors over ten slices: twenty appearances, or two variables.
assert_eq!(drifting, 20);
assert_eq!(career, 2);
assert_eq!(
drifting / career,
10,
"collapse should track the slice count"
);
}
/// With `drift = 0` consecutive appearances are the same latent variable, so
/// the joint is smaller than the appearance count.
#[test]
+338
View File
@@ -0,0 +1,338 @@
//! Configuring a competitor before anything is observed about them.
//!
//! The configuration a competitor needs is usually a property of the domain —
//! "every layout is static" — not of whichever event happens to mention them
//! first. Stating it per-event meant every ingestion path had to remember it,
//! and two of the four paths could not state it at all.
use smallvec::smallvec;
use trueskill_tt::{
ConstantDrift, ConvergenceOptions, Event, Gaussian, History, InferenceError, Member, Outcome,
Team,
};
type H = History<i64, ConstantDrift, trueskill_tt::NullObserver, &'static str>;
const PINNED: Gaussian = Gaussian::from_ms(2.0, 0.5);
fn history() -> H {
History::builder()
.mu(0.0)
.sigma(6.0)
.beta(1.0)
.score_sigma(2.0)
.drift(ConstantDrift(0.5))
.convergence(ConvergenceOptions {
max_iter: 20_000,
epsilon: 1e-13,
alpha: 1.0,
})
.build()
}
fn duel(
a: &'static str,
b: &'static str,
t: i64,
m: Option<Member<&'static str>>,
) -> Event<i64, &'static str> {
Event {
time: t,
teams: smallvec![
Team::with_members([Member::new(a)]),
Team::with_members([m.unwrap_or_else(|| Member::new(b))]),
],
outcome: Outcome::scores([5.0, 2.0]),
}
}
fn skills(h: &H) -> Vec<(&'static str, Gaussian)> {
["player", "layout"]
.into_iter()
.map(|k| (k, h.current_skill(&k).unwrap()))
.collect()
}
/// The headline contract.
#[test]
fn registering_matches_configuring_on_the_first_event() {
let configured = {
let mut h = history();
h.add_events(vec![
duel(
"player",
"layout",
1,
Some(
Member::new("layout")
.with_drift_scale(0.0)
.with_prior(PINNED),
),
),
duel("player", "layout", 2, None),
])
.unwrap();
let _ = h.converge().unwrap();
h
};
let registered = {
let mut h = history();
h.register(
Member::new("layout")
.with_drift_scale(0.0)
.with_prior(PINNED),
)
.unwrap();
h.add_events(vec![
duel("player", "layout", 1, None),
duel("player", "layout", 2, None),
])
.unwrap();
let _ = h.converge().unwrap();
h
};
for ((k, a), (_, b)) in skills(&configured).into_iter().zip(skills(&registered)) {
assert_eq!(a.pi(), b.pi(), "{k} pi");
assert_eq!(a.tau(), b.tau(), "{k} tau");
}
}
/// The case `EventBuilder` and the typed path cannot reach: a competitor whose
/// first appearance arrives through the two-argument convenience route.
#[test]
fn registration_reaches_a_competitor_first_seen_through_record_winner() {
let mut h = history();
h.register(
Member::new("layout")
.with_drift_scale(0.0)
.with_prior(PINNED),
)
.unwrap();
h.record_winner(&"player", &"layout", 1).unwrap();
h.record_winner(&"player", &"layout", 2).unwrap();
let _ = h.converge().unwrap();
let rating = h.rating(&"layout").unwrap();
assert_eq!(rating.drift_scale(), 0.0);
assert_eq!(rating.prior().mu(), PINNED.mu());
// Pinned means pinned: no drift across the two slices.
let curve = h.learning_curve(&"layout");
assert!(curve.len() >= 2);
let widest = curve
.iter()
.map(|(_, g)| g.sigma())
.fold(f64::MIN, f64::max);
let narrowest = curve
.iter()
.map(|(_, g)| g.sigma())
.fold(f64::MAX, f64::min);
assert!(
(widest - narrowest) / widest < 1e-9,
"{narrowest} .. {widest}"
);
}
#[test]
fn registering_a_known_competitor_is_an_error() {
let mut h = history();
h.record_winner(&"player", &"layout", 1).unwrap();
let err = h.register(Member::new("layout")).unwrap_err();
assert!(
matches!(err, InferenceError::AlreadyRegistered { .. }),
"{err:?}"
);
}
#[test]
fn registering_twice_is_an_error() {
let mut h = history();
h.register(Member::new("layout").with_drift_scale(0.0))
.unwrap();
let err = h
.register(Member::new("layout").with_drift_scale(1.0))
.unwrap_err();
assert!(
matches!(err, InferenceError::AlreadyRegistered { .. }),
"{err:?}"
);
// The first registration stands.
assert_eq!(h.rating(&"layout").unwrap().drift_scale(), 0.0);
}
/// `weight` is per-event and meaningless here, so it is rejected rather than
/// dropped — dropping it silently is the defect class this whole area keeps
/// producing.
#[test]
fn a_weight_on_a_registration_is_rejected() {
let mut h = history();
let err = h
.register(Member::new("layout").with_weight(0.5))
.unwrap_err();
assert!(
matches!(err, InferenceError::InvalidParameter { name: "weight", .. }),
"{err:?}"
);
}
#[test]
fn an_invalid_drift_scale_on_a_registration_is_rejected() {
for bad in [-1.0, f64::NAN, f64::INFINITY] {
let mut h = history();
let err = h
.register(Member::new("layout").with_drift_scale(bad))
.unwrap_err();
assert!(
matches!(
err,
InferenceError::InvalidParameter {
name: "drift_scale",
..
}
),
"{bad}: {err:?}"
);
}
}
/// Registration makes the fit independent of the order events arrive in,
/// which is what the per-event shape could not guarantee.
#[test]
fn registration_makes_the_fit_order_independent() {
let build = |reversed: bool| {
let mut h = history();
h.register(
Member::new("layout")
.with_drift_scale(0.0)
.with_prior(PINNED),
)
.unwrap();
let mut events = vec![
duel("player", "layout", 1, None),
duel("player", "layout", 2, None),
duel("player", "layout", 3, None),
];
if reversed {
events.reverse();
}
h.add_events(events).unwrap();
let _ = h.converge().unwrap();
h
};
let forward = build(false);
let backward = build(true);
for ((k, a), (_, b)) in skills(&forward).into_iter().zip(skills(&backward)) {
assert_eq!(a.pi(), b.pi(), "{k} pi");
assert_eq!(a.tau(), b.tau(), "{k} tau");
}
}
/// `rating` is the read-back that made a configuration mistake detectable from
/// outside the crate at all. Every other accessor reports what inference
/// inferred; this reports what it was told.
#[test]
fn rating_reads_back_what_was_stored() {
let mut h = history();
assert!(h.rating(&"nobody").is_none());
h.register(
Member::new("layout")
.with_drift_scale(0.25)
.with_prior(PINNED),
)
.unwrap();
let r = h.rating(&"layout").unwrap();
assert_eq!(r.drift_scale(), 0.25);
assert_eq!(r.prior().pi(), PINNED.pi());
assert_eq!(r.prior().tau(), PINNED.tau());
// A competitor created by an event reports the history defaults.
h.record_winner(&"player", &"layout", 1).unwrap();
assert_eq!(h.rating(&"player").unwrap().drift_scale(), 1.0);
}
/// The decision this issue turned on: two different values for one competitor
/// are an error whether they arrive in one batch or two.
///
/// Last-write-wins across batches cut against the invariant
/// `tests/ingestion_equivalence.rs` protects — the same contradictory events
/// errored when batched and succeeded, order-dependently, one at a time.
mod conflicting_configuration {
use super::*;
fn seed(scale: f64) -> Event<i64, &'static str> {
duel(
"player",
"layout",
1,
Some(Member::new("layout").with_drift_scale(scale)),
)
}
#[test]
fn within_one_batch_is_an_error() {
let mut h = history();
let err = h.add_events(vec![seed(0.0), seed(1.0)]).unwrap_err();
assert!(
matches!(
err,
InferenceError::ConflictingCompetitorConfig {
field: "drift_scale",
..
}
),
"{err:?}"
);
}
#[test]
fn across_two_batches_is_also_an_error() {
let mut h = history();
h.add_events(vec![seed(0.0)]).unwrap();
let err = h.add_events(vec![seed(1.0)]).unwrap_err();
assert!(
matches!(
err,
InferenceError::ConflictingCompetitorConfig {
field: "drift_scale",
..
}
),
"{err:?}"
);
// Rejected before anything mutates: the first declaration stands.
assert_eq!(h.rating(&"layout").unwrap().drift_scale(), 0.0);
}
/// Repeating the *same* value stays inert, which is the expected shape
/// when the configuration is a property of the domain.
#[test]
fn repeating_the_same_value_is_inert() {
let mut h = history();
h.add_events(vec![seed(0.0)]).unwrap();
h.add_events(vec![seed(0.0)]).unwrap();
assert_eq!(h.rating(&"layout").unwrap().drift_scale(), 0.0);
}
/// A registration and a later event that agree are fine; one that
/// disagrees is the same error.
#[test]
fn a_registration_conflicts_with_a_later_event() {
let mut h = history();
h.register(Member::new("layout").with_drift_scale(0.0))
.unwrap();
h.add_events(vec![seed(0.0)]).unwrap();
let mut h2 = history();
h2.register(Member::new("layout").with_drift_scale(0.0))
.unwrap();
let err = h2.add_events(vec![seed(1.0)]).unwrap_err();
assert!(
matches!(err, InferenceError::ConflictingCompetitorConfig { .. }),
"{err:?}"
);
}
}
+71
View File
@@ -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:?}"
);
}
}