Measured, a NaN weight behaved exactly as `0.0`:
weight NaN -> Ok, converged: true, 1 iteration, step (0.0, 0.0)
skill pi 0.027777777777777776, tau 0.0
weight 0.0 -> Ok, same skill, bit for bit
So a NaN arriving from a division or a parse was indistinguishable from
a deliberate zero, and the fit reported itself as cleanly converged.
Worth correcting an earlier description of this: the event does not
vanish. The member contributes nothing, which is precisely what weight
zero means, and that equivalence is what makes it undetectable rather
than merely wrong.
Zero and negative weights stay accepted. Both are expressible choices
about how much a member contributes, and tests/degenerate_inputs.rs pins
their behaviour deliberately; only values that are not quantities at all
are rejected. A test asserts they still ingest, so the new check cannot
quietly widen.
This completes the boundary: every malformed input that previously
produced a plausible answer — a one-team event, an empty team, a
non-finite score, a non-finite weight — now fails where it enters.
BREAKING CHANGE: an event carrying a non-finite weight returns
`InvalidParameter` instead of silently treating that member as weightless.
Refs #18
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
191 lines
6.1 KiB
Rust
191 lines
6.1 KiB
Rust
//! Malformed events must be rejected at the ingestion boundary.
|
|
//!
|
|
//! Every case here was reachable from safe public API in a release build. Two
|
|
//! of them are the two shapes this crate's defects keep taking: a panic from
|
|
//! deep inside inference, and a finite, plausible-looking posterior computed
|
|
//! from an event that should never have been accepted.
|
|
//!
|
|
//! `InferenceError::NotEnoughTeams` and `EmptyTeam` already existed when these
|
|
//! were found — they were checked on the prediction paths and nowhere else, so
|
|
//! ingestion could still manufacture the states they describe.
|
|
|
|
use smallvec::smallvec;
|
|
use trueskill_tt::{Event, History, InferenceError, Member, Outcome, Team};
|
|
|
|
type Ev = Event<i64, &'static str>;
|
|
|
|
fn history() -> History<i64, trueskill_tt::ConstantDrift, trueskill_tt::NullObserver, &'static str>
|
|
{
|
|
History::builder().score_sigma(1.0).build()
|
|
}
|
|
|
|
fn teams(names: &[&[&'static str]]) -> smallvec::SmallVec<[Team<&'static str>; 4]> {
|
|
names
|
|
.iter()
|
|
.map(|team| Team::with_members(team.iter().map(|k| Member::new(*k))))
|
|
.collect()
|
|
}
|
|
|
|
/// The regression this file exists for: `run_chain` builds one diff link per
|
|
/// adjacent pair of teams, so a one-team event left it indexing `links[1..]`
|
|
/// on an empty vector and panicked — in release, from `History::add_events`.
|
|
#[test]
|
|
fn a_one_team_event_is_an_error_not_a_panic() {
|
|
let mut h = history();
|
|
let err = h
|
|
.add_events(vec![Ev {
|
|
time: 1,
|
|
teams: teams(&[&["a"]]),
|
|
outcome: Outcome::winner(0, 1),
|
|
}])
|
|
.unwrap_err();
|
|
assert!(
|
|
matches!(err, InferenceError::NotEnoughTeams { got: 1 }),
|
|
"{err:?}"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn a_zero_team_event_is_an_error() {
|
|
let mut h = history();
|
|
let err = h
|
|
.add_events(vec![Ev {
|
|
time: 1,
|
|
teams: smallvec![],
|
|
outcome: Outcome::ranking([]),
|
|
}])
|
|
.unwrap_err();
|
|
assert!(
|
|
matches!(err, InferenceError::NotEnoughTeams { got: 0 }),
|
|
"{err:?}"
|
|
);
|
|
}
|
|
|
|
/// The quiet half. An empty team contributes no performance, so before this
|
|
/// was rejected the event converged and handed back a finite posterior for its
|
|
/// opponent — a plausible constant computed from nothing.
|
|
#[test]
|
|
fn an_empty_team_is_an_error_rather_than_a_free_win() {
|
|
let mut h = history();
|
|
let err = h
|
|
.add_events(vec![Ev {
|
|
time: 1,
|
|
teams: teams(&[&[], &["b"]]),
|
|
outcome: Outcome::winner(0, 2),
|
|
}])
|
|
.unwrap_err();
|
|
assert!(
|
|
matches!(err, InferenceError::EmptyTeam { team: 0 }),
|
|
"{err:?}"
|
|
);
|
|
// Nothing was recorded, so the history is still empty.
|
|
assert!(h.current_skill(&"b").is_none());
|
|
}
|
|
|
|
#[test]
|
|
fn an_empty_team_is_reported_by_position() {
|
|
let mut h = history();
|
|
let err = h
|
|
.add_events(vec![Ev {
|
|
time: 1,
|
|
teams: teams(&[&["a"], &[]]),
|
|
outcome: Outcome::winner(0, 2),
|
|
}])
|
|
.unwrap_err();
|
|
assert!(
|
|
matches!(err, InferenceError::EmptyTeam { team: 1 }),
|
|
"{err:?}"
|
|
);
|
|
}
|
|
|
|
/// A NaN score used to ingest cleanly. `converge` reported `NonFiniteResult`,
|
|
/// but a caller who read `current_skill` first was handed `tau: NaN` with
|
|
/// nothing to say so.
|
|
#[test]
|
|
fn a_non_finite_score_is_rejected_at_ingestion() {
|
|
for bad in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] {
|
|
let mut h = history();
|
|
let err = h
|
|
.add_events(vec![Ev {
|
|
time: 1,
|
|
teams: teams(&[&["a"], &["b"]]),
|
|
outcome: Outcome::scores([bad, 0.0]),
|
|
}])
|
|
.unwrap_err();
|
|
assert!(
|
|
matches!(err, InferenceError::InvalidParameter { name: "score", .. }),
|
|
"{bad}: {err:?}"
|
|
);
|
|
assert!(h.current_skill(&"a").is_none(), "{bad} was recorded anyway");
|
|
}
|
|
}
|
|
|
|
/// A non-finite weight behaved exactly as `0.0` — the member contributed
|
|
/// nothing — while `converge` reported `converged: true` after one iteration
|
|
/// with a step of `(0.0, 0.0)`. So a NaN arriving from a division or a parse
|
|
/// was indistinguishable from a deliberate zero, and looked like a clean fit.
|
|
#[test]
|
|
fn a_non_finite_weight_is_rejected_at_ingestion() {
|
|
for bad in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] {
|
|
let mut h = history();
|
|
let err = h
|
|
.event(1)
|
|
.team(["a"])
|
|
.weights([bad])
|
|
.team(["b"])
|
|
.winner(0)
|
|
.commit()
|
|
.unwrap_err();
|
|
assert!(
|
|
matches!(err, InferenceError::InvalidParameter { name: "weight", .. }),
|
|
"{bad}: {err:?}"
|
|
);
|
|
assert!(h.current_skill(&"a").is_none(), "{bad} reached the history");
|
|
}
|
|
}
|
|
|
|
/// Zero and negative weights are expressible choices about how much a member
|
|
/// contributes, not malformed input, and `tests/degenerate_inputs.rs` pins
|
|
/// their behaviour deliberately. Rejecting non-finite values must not catch
|
|
/// them too.
|
|
#[test]
|
|
fn zero_and_negative_weights_still_ingest() {
|
|
for w in [0.0, -1.0, 0.5] {
|
|
let mut h = history();
|
|
h.event(1)
|
|
.team(["a"])
|
|
.weights([w])
|
|
.team(["b"])
|
|
.winner(0)
|
|
.commit()
|
|
.unwrap_or_else(|e| panic!("weight {w} should ingest: {e:?}"));
|
|
assert!(h.current_skill(&"a").is_some(), "weight {w}");
|
|
}
|
|
}
|
|
|
|
/// The fluent builder routes through the same chokepoint, so it inherits the
|
|
/// checks rather than needing its own.
|
|
#[test]
|
|
fn the_event_builder_inherits_the_shape_checks() {
|
|
let mut h = history();
|
|
let err = h.event(1).team(["a"]).winner(0).commit().unwrap_err();
|
|
assert!(
|
|
matches!(err, InferenceError::NotEnoughTeams { got: 1 }),
|
|
"{err:?}"
|
|
);
|
|
}
|
|
|
|
/// A well-formed event is untouched by any of this.
|
|
#[test]
|
|
fn a_well_formed_event_still_ingests() {
|
|
let mut h = history();
|
|
h.add_events(vec![Ev {
|
|
time: 1,
|
|
teams: teams(&[&["a"], &["b"]]),
|
|
outcome: Outcome::scores([3.0, 1.0]),
|
|
}])
|
|
.unwrap();
|
|
assert!(h.converge().unwrap().converged);
|
|
assert!(h.current_skill(&"a").unwrap().mu() > h.current_skill(&"b").unwrap().mu());
|
|
}
|