diff --git a/src/error.rs b/src/error.rs index d0dfb01..de9d905 100644 --- a/src/error.rs +++ b/src/error.rs @@ -118,6 +118,17 @@ pub enum InferenceError { member: usize, key: String, }, + /// `History::register` was called for a competitor that already exists. + /// + /// Registration states a competitor's configuration before anything has + /// been observed about them, so a competitor that already exists has + /// already been configured — by an earlier `register`, or by an event that + /// created them. Silently overwriting would reintroduce exactly the + /// order-dependence registration exists to remove. + /// + /// To change an existing competitor's configuration, supply it on an event + /// through `Member`; that refits the whole history. + AlreadyRegistered { key: String }, /// A prediction was given a team with no members. EmptyTeam { team: usize }, /// A joint posterior was requested where one cannot be formed exactly. @@ -197,6 +208,14 @@ impl fmt::Display for InferenceError { with `lookup` or `current_skill` if that is not guaranteed)" ) } + Self::AlreadyRegistered { key } => { + write!( + f, + "competitor {key} is already registered; registration states \ + configuration before anything is observed, so re-registering \ + would silently overwrite it" + ) + } Self::EmptyTeam { team } => { write!(f, "team {team} has no members") } diff --git a/src/history.rs b/src/history.rs index 40befe9..c769d97 100644 --- a/src/history.rs +++ b/src/history.rs @@ -6,6 +6,7 @@ use crate::{ convergence::{ConvergenceOptions, ConvergenceReport}, drift::{ConstantDrift, Drift}, error::InferenceError, + event::Member, gaussian::Gaussian, key_table::KeyTable, observer::{NullObserver, Observer}, @@ -207,6 +208,7 @@ impl, O: Observer, K: Eq + Hash + Clone> HistoryBuilder< convergence: self.convergence, observer: self.observer, unknown_keys: self.unknown_keys, + declared: HashMap::new(), } } } @@ -314,6 +316,12 @@ pub struct History< convergence: ConvergenceOptions, observer: O, unknown_keys: crate::UnknownKeys, + /// Competitor configuration explicitly declared so far, by whichever route. + /// + /// Kept separate from the applied `Rating` because a `Rating` cannot say + /// whether a value was *chosen* or inherited from the history defaults, + /// and that is exactly the distinction a conflict check needs. + declared: HashMap, } impl Default for History { @@ -493,6 +501,111 @@ impl, O: Observer, K: Eq + Hash + Clone> History(()) + /// ``` + /// + /// The competitor exists from this point on, with no appearances, so + /// [`History::rating`] can read back what was actually stored — the + /// diagnostic that was previously missing entirely. + /// + /// `weight` is per-event and has no meaning here, so a `Member` carrying a + /// non-default one is rejected rather than silently ignored. + /// + /// # Errors + /// + /// `AlreadyRegistered` if the competitor already exists, whether from an + /// earlier `register` or from an event. `InvalidParameter` for a `weight` + /// other than 1.0, or a `drift_scale` that is negative or non-finite. + pub fn register(&mut self, member: Member) -> Result<(), InferenceError> + where + K: std::fmt::Debug, + { + if member.weight != 1.0 { + return Err(InferenceError::InvalidParameter { + name: "weight", + value: member.weight, + }); + } + if let Some(scale) = member.drift_scale { + if !scale.is_finite() || scale < 0.0 { + return Err(InferenceError::InvalidParameter { + name: "drift_scale", + value: scale, + }); + } + } + + let key = format!("{:?}", member.key); + let idx = self.keys.get_or_create(&member.key); + if self.agents.contains(idx) { + return Err(InferenceError::AlreadyRegistered { key }); + } + + let mut rating = Rating::new( + Gaussian::from_ms(self.mu, self.sigma), + self.beta, + self.drift, + ); + if let Some(prior) = member.prior { + rating.prior = prior; + } + if let Some(scale) = member.drift_scale { + rating.drift_scale = scale; + } + + self.declared.insert( + idx, + CompetitorConfig { + prior: member.prior, + drift_scale: member.drift_scale, + }, + ); + self.agents.insert( + idx, + Competitor { + rating, + message: None, + last_time: None, + }, + ); + + Ok(()) + } + + /// The configuration in force for a competitor, or `None` if the history + /// has never seen them. + /// + /// Reads back what was actually stored, which is what makes a + /// configuration mistake detectable from outside the crate. Every other + /// accessor returns what inference *inferred*; this returns what it was + /// told. + #[must_use] + pub fn rating(&self, key: &Q) -> Option> + where + K: std::borrow::Borrow, + Q: std::hash::Hash + Eq + ?Sized, + { + let idx = self.keys.get(key)?; + self.agents.contains(idx).then(|| self.agents[idx].rating) + } + pub fn current_skill(&self, key: &Q) -> Option where K: std::borrow::Borrow, @@ -1647,6 +1760,47 @@ impl, O: Observer, K: Eq + Hash + Clone> History, O: Observer, K: Eq + Hash + Clone> History; + +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(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>, +) -> Event { + 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(®istered)) { + 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 { + 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:?}" + ); + } +}