`Member::with_prior` and `with_drift_scale` were consumed only on the branch that *creates* a competitor — `priors.remove` sat inside `if !self.agents.contains(..)`. Supplying either for a key the history already knew did nothing at all: no error, no warning, and output computed from the default prior. A prior applied on a competitor's very first event and was silently discarded ever after. Configuration now applies whenever supplied. Two details this forced: Configuration is tracked per *field* rather than as a merged `Rating`. A member setting only `drift_scale` must not also assert the default prior, or it would silently undo a prior seeded on an earlier event. Slice state has to be refreshed. `drift_scale` is re-derived on every forward pass, but a prior is written into the competitor's earliest slice once, at ingestion, and `iteration` refreshes only slices after the first. Without the refresh a late prior would reach the drift terms and nothing else — a subtler version of the drop being fixed. This was caught by a test, not by reading the code. Conflicting values for one competitor within a single batch are now `ConflictingCompetitorConfig` rather than resolved by iteration order. Events in a batch are unordered, so "last one wins" would make the result depend on traversal — and `tests/ingestion_equivalence.rs` exists to rule exactly that out. Repeating the same value stays inert, which is the shape callers get when configuration is a property of the domain. That invariant turned out to be tested only for *unconfigured* competitors: every helper in that file built members with `Member::new`. Extended to cover configured ones, including a check that configuration changes the fit at all, so the order tests cannot pass vacuously. `with_prior` had no coverage under `tests/` whatsoever, which is how this survived. Adds `tests/competitor_config.rs`. `drift_scale_is_ignored_after_first_appearance` asserted the old behaviour and now asserts the new one. It was written as a deliberate change-detector — "moving the capture would be a visible break, not a silent one" — so it inverted rather than being deleted. Also removes `InferenceError::ConvergenceFailed` and `NegativePrecision`, which no code path ever constructed: public variants advertising failure modes no caller could observe. Partial #20 — its other items were already resolved, except `Outcome::winner` still panicking. BREAKING CHANGE: `prior` and `drift_scale` now take effect for competitors the history already knows, where they were previously ignored; a batch supplying conflicting values for one competitor is now an error. `InferenceError::ConvergenceFailed` and `InferenceError::NegativePrecision` are removed. Closes #10. Refs #20. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
226 lines
7.1 KiB
Rust
226 lines
7.1 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_with_key().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"
|
|
);
|
|
}
|
|
}
|