Files
trueskill-tt/tests/registration.rs
T
logaritmiskandClaude Opus 5 8dff7513f7 fix!: seal ConstantDrift's field so gamma can be validated
`gamma` enters only as `gamma * gamma`, so the sign was squared away:
measured against the old public-field form, `ConstantDrift(-0.0833)`
produced results bit identical to `ConstantDrift(0.0833)`. The sign was
neither rejected nor honoured — it vanished.

It could not be checked while the field was a public tuple position,
because there was nothing to intercept. Validating inside
`variance_for_elapsed` would have been worse: it runs in 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 — rejecting NaN there
turned the NonFiniteResult reporting path into a crash.

So `ConstantDrift::new` is the only way in and it checks, with `gamma()`
to read the value back. 129 call sites rewritten across src, tests,
benches, examples and the README. The dated plan and spec documents under
docs/superpowers are left alone: they record what was built at the time,
and rewriting them would falsify that.

tests/constructor_validation.rs is the more valuable half. This defect
class was closed three times in one session and reopened twice, because
each fix validated the layer it had just touched and inferred the rest —
`HistoryBuilder`, then `Game`'s own entry points, then the constructors
beneath both. A per-site fix cannot notice the site nobody thought of, so
that file enumerates every public entry point taking a magnitude and
asserts each refuses negative and non-finite values.

It found an eleventh defect on its first run: `HistoryBuilder::score_sigma`
accepted infinity, because `inf > 0.0` is true and the assert only tested
positivity. Fixed, and its own `should_panic` message updated to match.

`Gaussian::from_ms` is deliberately exempt from the non-finite half, for
the reason above: a broken fit produces a NaN sigma legitimately and
`converge` must be allowed to report it.

The convergence-level drift-variance check stays and is now tested through
a custom `Drift` implementation, since `ConstantDrift` can no longer reach
it. That check is the only thing standing between a third-party `Drift`
and a NaN fit.

BREAKING CHANGE: `ConstantDrift`'s field is private. Replace
`ConstantDrift(x)` with `ConstantDrift::new(x)`, and `drift().0` with
`drift().gamma()`. `HistoryBuilder::score_sigma` now rejects infinity.

Closes #65

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
2026-09-09 19:11:57 +02:00

339 lines
9.9 KiB
Rust

//! 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::new(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:?}"
);
}
}