`History<T: Time, ..>` has always been generic over the time axis,
`Untimed` has always been exported, and `Drift<T>` is generic specifically
so that "seasonal or calendar-aware drift is expressible without going
through i64". None of it was reachable from a downstream crate.
Every construction route pinned `T = i64`: `History::builder()`,
`History::builder_with_key()`, and the only `Default` impl on
`HistoryBuilder`. Its fields are private and it had no `new`. So all three
escape routes failed to compile, and a consumer with domain timestamps
had to convert to i64 — which is the exact thing the parameter exists to
avoid. One of `History`'s four type parameters was paid for at every
signature and could never be varied.
`Default` is now generic over `T` and `K`, `HistoryBuilder::new()` exists,
and `time_type::<T2>()` / `key_type::<K2>()` join `drift` and `observer`
as type-changing setters:
History::builder().time_type::<Untimed>().build()
History::builder().key_type::<String>().build()
HistoryBuilder::<Season, _, _, String>::new().build()
`key_type` replaces `builder_with_key`, which could not be turbofished —
`K` sat on the impl rather than the function, so callers had to spell
`History::<i64, _, _, String>::builder_with_key()`. 18 call sites across
15 files migrated.
tests/time_axis.rs is the part that matters. NOTHING in the repository
constructed a non-i64 history, which is precisely why this survived, so
the fix is only half done without a test that exercises the generic. It
defines a `Season(u16)` time type and a `SeasonalDrift` that accumulates
between seasons but not within one — the calendar-aware case the trait's
docs cite — and checks the whole path: fit, converge, and read a learning
curve whose times come back as `Season`, not as integers.
Two of the six tests are controls rather than assertions about output.
`Untimed` must ignore drift entirely, since elapsed is always zero, so
gamma 0.0 and gamma 5.0 must agree bit for bit. And a custom `Drift` must
actually widen a gap across seasons, or the test above would pass whether
or not the drift was consulted at all.
The README's ticked "Generalise a time axis" box is now true.
BREAKING CHANGE: `History::builder_with_key()` is removed. Use
`History::builder().key_type::<K>()`.
Closes #68
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
228 lines
7.2 KiB
Rust
228 lines
7.2 KiB
Rust
//! Ingesting the same events must give the same answer however they were
|
|
//! batched.
|
|
//!
|
|
//! The numerical goldens all ingest in a single call with one slice per
|
|
//! timestamp, so they never exercise the "append to an existing slice" path.
|
|
//! These do.
|
|
|
|
use smallvec::smallvec;
|
|
use trueskill_tt::{ConvergenceOptions, Event, Gaussian, History, Member, Outcome, Team};
|
|
|
|
/// Converge tightly: the default cap of 30 iterations leaves a residual around
|
|
/// 1e-6, which would swamp the comparison. Both paths must reach the same
|
|
/// fixed point, so drive both well past it.
|
|
fn tight() -> ConvergenceOptions {
|
|
ConvergenceOptions {
|
|
max_iter: 2_000,
|
|
epsilon: 1e-12,
|
|
..ConvergenceOptions::default()
|
|
}
|
|
}
|
|
|
|
fn event(a: &str, b: &str, time: i64) -> Event<i64, String> {
|
|
Event {
|
|
time,
|
|
teams: smallvec![
|
|
Team::with_members([Member::new(a.to_string())]),
|
|
Team::with_members([Member::new(b.to_string())]),
|
|
],
|
|
outcome: Outcome::winner(0, 2),
|
|
}
|
|
}
|
|
|
|
/// Like [`event`], but `a` carries competitor configuration.
|
|
///
|
|
/// `prior` and `drift_scale` configure the competitor rather than the event, so
|
|
/// they are the part of ingestion most exposed to order: they are consumed once,
|
|
/// where the competitor's state is written.
|
|
fn configured_event(a: &str, b: &str, time: i64, scale: f64) -> Event<i64, String> {
|
|
Event {
|
|
time,
|
|
teams: smallvec![
|
|
Team::with_members([Member::new(a.to_string()).with_drift_scale(scale)]),
|
|
Team::with_members([Member::new(b.to_string())]),
|
|
],
|
|
outcome: Outcome::winner(0, 2),
|
|
}
|
|
}
|
|
|
|
fn converged_skills(events: Vec<Event<i64, String>>, batched: bool) -> Vec<(String, Gaussian)> {
|
|
let mut h: History<i64, _, _, String> = History::builder()
|
|
.key_type::<String>()
|
|
.convergence(tight())
|
|
.build();
|
|
|
|
if batched {
|
|
h.add_events(events).unwrap();
|
|
} else {
|
|
for ev in events {
|
|
h.add_events(std::iter::once(ev)).unwrap();
|
|
}
|
|
}
|
|
|
|
let report = h.converge().unwrap();
|
|
assert!(
|
|
report.converged,
|
|
"fixture must converge before results can be compared; final step {:?}",
|
|
report.final_step
|
|
);
|
|
|
|
let mut skills: Vec<(String, Gaussian)> = h
|
|
.learning_curves()
|
|
.into_iter()
|
|
.map(|(key, curve)| (key, curve.last().unwrap().1))
|
|
.collect();
|
|
skills.sort_by(|a, b| a.0.cmp(&b.0));
|
|
skills
|
|
}
|
|
|
|
fn assert_same(batched: &[(String, Gaussian)], incremental: &[(String, Gaussian)], what: &str) {
|
|
assert_eq!(
|
|
batched.len(),
|
|
incremental.len(),
|
|
"{what}: competitor count differs"
|
|
);
|
|
|
|
for ((kb, gb), (ki, gi)) in batched.iter().zip(incremental.iter()) {
|
|
assert_eq!(kb, ki, "{what}: key order differs");
|
|
assert!(
|
|
(gb.mu() - gi.mu()).abs() < 1e-8 && (gb.sigma() - gi.sigma()).abs() < 1e-8,
|
|
"{what}: {kb} differs — batched mu={} sigma={}, incremental mu={} sigma={}",
|
|
gb.mu(),
|
|
gb.sigma(),
|
|
gi.mu(),
|
|
gi.sigma()
|
|
);
|
|
}
|
|
}
|
|
|
|
/// All events share one timestamp, so incremental ingestion repeatedly appends
|
|
/// to an existing slice.
|
|
#[test]
|
|
fn same_slice_incremental_matches_batched() {
|
|
let events = vec![
|
|
event("a", "b", 1),
|
|
event("c", "d", 1),
|
|
event("e", "f", 1),
|
|
event("a", "c", 1),
|
|
event("b", "e", 1),
|
|
];
|
|
|
|
let batched = converged_skills(events.clone(), true);
|
|
let incremental = converged_skills(events, false);
|
|
assert_same(&batched, &incremental, "single shared slice");
|
|
}
|
|
|
|
/// Distinct timestamps, so each append lands in a fresh slice appended after
|
|
/// the existing ones.
|
|
#[test]
|
|
fn distinct_slices_incremental_matches_batched() {
|
|
let events = vec![
|
|
event("a", "b", 1),
|
|
event("b", "c", 2),
|
|
event("c", "a", 3),
|
|
event("a", "c", 4),
|
|
];
|
|
|
|
let batched = converged_skills(events.clone(), true);
|
|
let incremental = converged_skills(events, false);
|
|
assert_same(&batched, &incremental, "distinct slices");
|
|
}
|
|
|
|
/// Several events per timestamp across several timestamps — appends to
|
|
/// existing slices interleaved with new ones.
|
|
#[test]
|
|
fn mixed_slices_incremental_matches_batched() {
|
|
let events = vec![
|
|
event("a", "b", 1),
|
|
event("c", "d", 1),
|
|
event("a", "c", 2),
|
|
event("b", "d", 2),
|
|
event("a", "d", 3),
|
|
event("b", "c", 3),
|
|
];
|
|
|
|
let batched = converged_skills(events.clone(), true);
|
|
let incremental = converged_skills(events, false);
|
|
assert_same(&batched, &incremental, "mixed slices");
|
|
}
|
|
|
|
/// Appending an event to a slice that is *not* the most recent one exercises
|
|
/// the forward refresh of every later slice.
|
|
#[test]
|
|
fn back_dated_event_matches_batched() {
|
|
let events = vec![
|
|
event("a", "b", 1),
|
|
event("b", "c", 5),
|
|
event("c", "a", 9),
|
|
// arrives last, but belongs to the middle slice
|
|
event("a", "c", 5),
|
|
];
|
|
|
|
let batched = converged_skills(events.clone(), true);
|
|
let incremental = converged_skills(events, false);
|
|
assert_same(&batched, &incremental, "back-dated event");
|
|
}
|
|
|
|
/// The invariant this file protects was only ever checked for *unconfigured*
|
|
/// competitors — every helper above built members with `Member::new`.
|
|
///
|
|
/// Configuration is the part most exposed to ordering, because it is consumed
|
|
/// once at the point the competitor's state is written rather than replayed per
|
|
/// event. These cover it.
|
|
#[test]
|
|
fn configured_competitors_are_order_independent() {
|
|
let events = vec![
|
|
configured_event("a", "b", 0, 0.0),
|
|
configured_event("a", "c", 1, 0.0),
|
|
configured_event("a", "b", 2, 0.0),
|
|
event("b", "c", 3),
|
|
];
|
|
|
|
assert_same(
|
|
&converged_skills(events.clone(), true),
|
|
&converged_skills(events, false),
|
|
"configuration repeated on every appearance",
|
|
);
|
|
}
|
|
|
|
/// Configuration supplied only on a *later* event is the case that used to be
|
|
/// silently dropped. It must now reach the same fit either way it is ingested.
|
|
#[test]
|
|
fn late_configuration_is_order_independent() {
|
|
let events = vec![
|
|
event("a", "b", 0),
|
|
configured_event("a", "c", 1, 0.0),
|
|
event("a", "b", 2),
|
|
];
|
|
|
|
assert_same(
|
|
&converged_skills(events.clone(), true),
|
|
&converged_skills(events, false),
|
|
"configuration supplied after first appearance",
|
|
);
|
|
}
|
|
|
|
/// And it must actually be doing something — an implementation that dropped
|
|
/// configuration entirely would pass both tests above.
|
|
#[test]
|
|
fn configuration_changes_the_fit_however_it_is_ingested() {
|
|
let configured = vec![
|
|
event("a", "b", 0),
|
|
configured_event("a", "c", 1, 0.0),
|
|
event("a", "b", 2),
|
|
];
|
|
let plain = vec![event("a", "b", 0), event("a", "c", 1), event("a", "b", 2)];
|
|
|
|
for batched in [true, false] {
|
|
let with = converged_skills(configured.clone(), batched);
|
|
let without = converged_skills(plain.clone(), batched);
|
|
assert!(
|
|
with.iter()
|
|
.zip(&without)
|
|
.any(|((_, x), (_, y))| (x.sigma() - y.sigma()).abs() > 1e-9),
|
|
"batched={batched}: configuration had no effect, so the order tests are vacuous"
|
|
);
|
|
}
|
|
}
|