diff --git a/src/color_group.rs b/src/color_group.rs index 0d8fe96..83081ba 100644 --- a/src/color_group.rs +++ b/src/color_group.rs @@ -191,3 +191,121 @@ mod tests { assert_eq!(cg.total_events(), 4); } } + +#[cfg(test)] +mod properties { + use std::collections::HashSet; + + use proptest::prelude::*; + + use super::*; + + /// The property the whole parallel sweep rests on: two events sharing a + /// competitor must never land in the same color, because a color group is + /// run concurrently and two events touching one competitor would race. + /// + /// Hand-written cases cover the shapes someone thought of. This covers the + /// ones nobody did — the correctness of `sweep_color_groups` depends on it + /// holding for every input, not for five. + fn check(events: &[Vec]) { + let groups = color_greedy(events.len(), |ev| { + events[ev] + .iter() + .copied() + .map(Index::from) + .collect::>() + }); + + // Disjointness *between events* within a color. Deduplicated per + // event, because one event legitimately naming a competitor twice is + // not a collision — `color_greedy` collects each event's members into + // a set for exactly that reason. + for color in 0..groups.n_colors() { + let mut seen: HashSet = HashSet::new(); + for &ev in &groups.groups[color] { + let members: HashSet = events[ev].iter().copied().collect(); + for competitor in members { + assert!( + seen.insert(competitor), + "competitor {competitor} shared by two events in color {color}" + ); + } + } + } + + // Every event is assigned exactly once. Without this, a partition that + // dropped events would satisfy disjointness trivially. + let mut assigned: Vec = groups.groups.iter().flatten().copied().collect(); + assigned.sort_unstable(); + assert_eq!(assigned, (0..events.len()).collect::>()); + assert_eq!(groups.total_events(), events.len()); + + // No empty colors: one would waste a sweep and make `n_colors` + // misleading. + for (color, group) in groups.groups.iter().enumerate() { + assert!(!group.is_empty(), "color {color} is empty"); + } + + // Contiguity is not a property of `color_greedy` — it holds only after + // `recompute_color_groups` reorders the events so each color occupies + // one range. What must always hold is that the reorder is *possible*: + // relabelling events in group order yields contiguous groups. The + // parallel sweep slices `&mut` sub-ranges from those, so if this ever + // failed the reorder would produce overlapping ranges. + let mut next = 0usize; + let relabelled: Vec> = groups + .groups + .iter() + .map(|group| { + group + .iter() + .map(|_| { + let i = next; + next += 1; + i + }) + .collect() + }) + .collect(); + assert!(ColorGroups { groups: relabelled }.groups_are_contiguous()); + } + + proptest! { + #![proptest_config(ProptestConfig::with_cases(512))] + + /// Small competitor pool, so collisions are common and colors are + /// forced to multiply. + #[test] + fn colors_are_disjoint_on_a_dense_pool( + events in prop::collection::vec( + prop::collection::vec(0usize..6, 1..4), + 0..20, + ) + ) { + check(&events); + } + + /// Wide pool, so most events are independent and land in one color. + #[test] + fn colors_are_disjoint_on_a_sparse_pool( + events in prop::collection::vec( + prop::collection::vec(0usize..200, 1..6), + 0..30, + ) + ) { + check(&events); + } + + /// Repeated competitors within one event must not confuse the + /// member-set bookkeeping. + #[test] + fn colors_are_disjoint_with_repeated_members( + events in prop::collection::vec( + prop::collection::vec(0usize..3, 1..8), + 0..15, + ) + ) { + check(&events); + } + } +} diff --git a/tests/non_finite_results.rs b/tests/non_finite_results.rs new file mode 100644 index 0000000..8928431 --- /dev/null +++ b/tests/non_finite_results.rs @@ -0,0 +1,117 @@ +//! Inference must report numerical breakdown rather than call it convergence. +//! +//! The boundary rejects inputs that are *not numbers*, but finite inputs can +//! still overflow during inference — `beta.powi(2)` at 1e300 is infinite, and +//! infinity minus infinity is NaN. `NonFiniteResult` is the guard for that, and +//! it matters because the alternative is silent: NaN fails every comparison, so +//! a naive `step < epsilon` check reads a NaN step as *converged*. +//! +//! That is why the crate has `step_converged` / `step_is_finite` rather than +//! `!tuple_gt(..)`. These tests pin the guard from outside. + +use smallvec::smallvec; +use trueskill_tt::{Event, Gaussian, History, InferenceError, Member, Outcome, Team}; + +fn scored_fit( + sigma: f64, + beta: f64, + score_sigma: f64, + scores: [f64; 2], +) -> Result { + let mut h = History::builder() + .mu(0.0) + .sigma(sigma) + .beta(beta) + .score_sigma(score_sigma) + .build(); + h.add_events(vec![Event { + time: 1i64, + teams: smallvec![ + Team::with_members([Member::new("a")]), + Team::with_members([Member::new("b")]), + ], + outcome: Outcome::scores(scores), + }])?; + h.converge().map(|r| r.converged) +} + +/// Every one of these is built from finite, individually legal parameters. The +/// overflow happens inside inference, which is exactly the case the boundary +/// checks cannot catch. +/// +/// Matched rather than merely `is_err()`: an assertion that only checks "some +/// error" would keep passing if these started failing at the boundary for an +/// unrelated reason, and would then be testing nothing. +#[test] +fn overflow_during_inference_is_reported_not_hidden() { + let cases: [(&str, f64, f64, f64, [f64; 2]); 5] = [ + ("huge sigma", 1e300, 1.0, 1.0, [3.0, 1.0]), + ("huge beta", 6.0, 1e300, 1.0, [3.0, 1.0]), + ("tiny sigma", 1e-300, 1.0, 1.0, [3.0, 1.0]), + ("tiny score_sigma", 6.0, 1.0, 1e-300, [3.0, 1.0]), + ("huge scores", 6.0, 1.0, 1.0, [1e308, -1e308]), + ]; + + for (name, sigma, beta, score_sigma, scores) in cases { + match scored_fit(sigma, beta, score_sigma, scores) { + Err(InferenceError::NonFiniteResult { context, step }) => { + assert_eq!(context, "History::converge", "{name}"); + assert!( + !step.0.is_finite() || !step.1.is_finite(), + "{name}: reported NonFiniteResult with a finite step {step:?}" + ); + } + other => panic!("{name}: expected NonFiniteResult, got {other:?}"), + } + } +} + +/// The trap the invariant exists for: NaN fails every comparison, so a naive +/// `step < epsilon` test reads a NaN step as converged. A breakdown must never +/// come back as a successful fit. +#[test] +fn a_broken_fit_is_never_reported_as_converged() { + let mut h = History::builder().build(); + h.add_events(vec![Event { + time: 1i64, + teams: smallvec![ + Team::with_members([Member::new("a").with_prior(Gaussian::from_ms(1e300, 1e-300))]), + Team::with_members([Member::new("b")]), + ], + outcome: Outcome::winner(0, 2), + }]) + .unwrap(); + + let err = h.converge().unwrap_err(); + assert!( + matches!(err, InferenceError::NonFiniteResult { .. }), + "a breakdown must not be reported as convergence: {err:?}" + ); + + // `converge_partial` must not launder it into an `Ok` either — the + // permissive path is permissive about *stopping short*, not about NaN. + let mut h2 = History::builder().build(); + h2.add_events(vec![Event { + time: 1i64, + teams: smallvec![ + Team::with_members([Member::new("a").with_prior(Gaussian::from_ms(1e300, 1e-300))]), + Team::with_members([Member::new("b")]), + ], + outcome: Outcome::winner(0, 2), + }]) + .unwrap(); + assert!(matches!( + h2.converge_partial().unwrap_err(), + InferenceError::NonFiniteResult { .. } + )); +} + +/// The neighbouring case, so the tests above cannot pass by the fit simply +/// always failing: ordinary extreme-but-workable parameters still converge. +#[test] +fn merely_extreme_parameters_still_converge() { + assert!(scored_fit(1e6, 1.0, 1.0, [3.0, 1.0]).unwrap()); + assert!(scored_fit(1e-6, 1.0, 1.0, [3.0, 1.0]).unwrap()); + assert!(scored_fit(6.0, 1.0, 1e6, [3.0, 1.0]).unwrap()); + assert!(scored_fit(6.0, 1.0, 1.0, [1e150, -1e150]).unwrap()); +}