diff --git a/docs/superpowers/specs/2026-08-27-filtered-estimates-design.md b/docs/superpowers/specs/2026-08-27-filtered-estimates-design.md index 9015775..4f9385c 100644 --- a/docs/superpowers/specs/2026-08-27-filtered-estimates-design.md +++ b/docs/superpowers/specs/2026-08-27-filtered-estimates-design.md @@ -312,6 +312,18 @@ inert. Results agree to within convergence tolerance rather than exactly. Normalising the order in the scratch builder would buy bit-identity at the cost of diverging from what the real sweep does; not worth it. + + **Measured after implementation, this risk is smaller than stated.** + Flipping the scratch's `color_groups_dirty` from `true` to `false` + switches it between the grouped sweep (`sweep_color_groups`) and the + sequential fallback across its entire convergence loop — a far larger + perturbation than a permuted event order — and the ingestion-order + invariance test stays green at `1e-8` under `max_iter: 2_000`, + `epsilon: 1e-12`. EP reaches the same fixed point regardless of sweep + order once driven far enough. The tolerance caveat is correct but + conservative. Note the flag itself is load-bearing: with it `false` the + scratch would take the sequential path always, diverging from the + production sweep it exists to mirror. - **Divergence risk.** If `TimeSlice`'s sweep gains state that the scratch construction does not initialise, the pass silently reads a default. The scratch builder must construct `Skill` field-by-field diff --git a/src/color_group.rs b/src/color_group.rs index 8898fdd..0d8fe96 100644 --- a/src/color_group.rs +++ b/src/color_group.rs @@ -103,7 +103,6 @@ impl ColorGroups { /// `Index` values that event touches. The returned `ColorGroups` has one /// inner `Vec` per color, containing event indices in the order /// they were assigned. -#[allow(dead_code)] pub(crate) fn color_greedy(n_events: usize, index_set: F) -> ColorGroups where F: Fn(usize) -> I, diff --git a/src/history.rs b/src/history.rs index fec86aa..813ebc0 100644 --- a/src/history.rs +++ b/src/history.rs @@ -428,14 +428,28 @@ impl, O: Observer, K: Eq + Hash + Clone> History f64 { + /// Sum per-slice evidence. + /// + /// `forward` selects `skill.forward` as each event's prior instead of the + /// cavity. That is a genuine forward-only (filtering) quantity ONLY on a + /// history that has never been converged: `iteration` alternates backward + /// and forward sweeps, so from the second iteration onward the likelihood + /// feeding the forward message has already absorbed backward information. + /// For a filtering quantity that holds after convergence, use + /// `filtered_log_evidence`. + pub(crate) fn log_evidence_internal(&self, forward: bool, targets: &[Index]) -> f64 { + // Bound before the closure so it captures the store rather than all of + // `&self`: capturing `&History` would drag `KeyTable` in and demand + // `K: Sync` from every caller, which the key type need not satisfy. + let agents = &self.agents; + #[cfg(feature = "rayon")] { use rayon::prelude::*; let per_slice: Vec = self .time_slices .par_iter() - .map(|ts| ts.log_evidence(targets, forward, &self.agents)) + .map(|ts| ts.log_evidence(targets, forward, agents)) .collect(); per_slice.into_iter().sum() } @@ -443,19 +457,19 @@ impl, O: Observer, K: Eq + Hash + Clone> History f64 { + pub fn log_evidence(&self) -> f64 { self.log_evidence_internal(false, &[]) } /// Log-evidence restricted to time slices containing at least one of the /// given keys. Useful for leave-one-out cross-validation. - pub fn log_evidence_for(&mut self, keys: &[&Q]) -> f64 + pub fn log_evidence_for(&self, keys: &[&Q]) -> f64 where K: std::borrow::Borrow, Q: std::hash::Hash + Eq + ?Sized, diff --git a/src/time_slice.rs b/src/time_slice.rs index e9a9834..0704849 100644 --- a/src/time_slice.rs +++ b/src/time_slice.rs @@ -305,8 +305,9 @@ impl TimeSlice { *idx, Skill { forward: agents[*idx].receive(&self.time), + backward: N_INF, + likelihood: N_INF, elapsed, - ..Default::default() }, ); } diff --git a/tests/degenerate_inputs.rs b/tests/degenerate_inputs.rs index 1c4fa14..1a6bb16 100644 --- a/tests/degenerate_inputs.rs +++ b/tests/degenerate_inputs.rs @@ -5,7 +5,7 @@ use trueskill_tt::{ ConstantDrift, ConvergenceOptions, Game, GameOptions, Gaussian, History, InferenceError, - Outcome, Rating, + NullObserver, Outcome, Rating, }; type R = Rating; @@ -127,6 +127,20 @@ fn empty_history_converges_trivially() { assert!(report.converged); } +/// Issue #27's exact reproduction: a non-default key type reaching `converge` +/// with no events at all. The underflow it reported trapped in debug and +/// indexed out of bounds in release, so this must run in both profiles. +#[test] +fn converge_on_an_empty_history_with_owned_keys() { + let mut history: History = + History::builder_with_key().score_sigma(5.0).build(); + + let report = history.converge().unwrap(); + + assert_eq!(report.iterations, 0); + assert!(report.converged); +} + #[test] fn empty_event_stream_then_converge() { let mut h = History::default();