refactor!: remove the factor-graph surface nothing used, add try_winner
Two decisions taken before cutting 0.5.0. #42 — the `Schedule` trait was public API the engine never called. Its only call site was `Game::custom`, itself `#[doc(hidden)]`, and `EpsilonOrMax` was never constructed anywhere. Removing that surface showed the problem was larger than the issue described: with `custom` gone, the compiler found `Factor`, `BuiltinFactor`, `RankDiffFactor` and `TeamSumFactor` all dead too. `Game::run_chain` drives a local `DiffFactor` enum and bypasses the whole T1 abstraction — it has done since it was written. So this is not just an unused extension point but the machinery it was built on, and `CLAUDE.md` was documenting it as live architecture. Removed: `graph` module, `Schedule`, `EpsilonOrMax`, `ScheduleReport`, `Game::custom`, `Factor`, `BuiltinFactor`, `RankDiffFactor`, `TeamSumFactor`. `TruncFactor`, `MarginFactor`, `VarStore` and `VarId` stay — inference uses those. The measurement behind choosing removal over wiring is in #42: the within-game loop converges in 1 to 8 iterations against a cap of 30, so a `Residual` schedule has no headroom to reclaim, and `Damped` already shipped as `ConvergenceOptions::alpha`. #20 — `Outcome::winner` panicking on an out-of-range index. Kept, and the reasoning is now on the method. It is the only constructor here that validates, which looks inconsistent until you try deferring like its siblings: `winner(5, 2)` produces ranks `[1, 1]`, an all-tied draw that ingestion accepts without complaint when `p_draw > 0`. Asking "team 5 won" and silently getting "everyone drew" is the exact failure this crate keeps removing, so the check belongs where the mistake is. Adds `Outcome::try_winner` for indices that are computed or parsed rather than written literally, following the `new`/`try_new` convention. That is additive; the panicking form stays because every call site in this repo, its tests and its README passes literals, where a `?` would be noise. BREAKING CHANGE: the `graph` module and everything it exported are removed, as are `Game::custom` and `ScheduleReport`. Closes #42. Closes #20. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
use crate::{
|
||||
N_INF,
|
||||
factor::{Factor, VarId, VarStore},
|
||||
factor::{VarId, VarStore},
|
||||
gaussian::Gaussian,
|
||||
ln_pdf,
|
||||
};
|
||||
@@ -55,12 +55,16 @@ impl MarginFactor {
|
||||
}
|
||||
}
|
||||
|
||||
impl Factor for MarginFactor {
|
||||
fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64) {
|
||||
/// Undamped wrappers, used by this module's tests. Inference drives these
|
||||
/// factors through `propagate_with_alpha` and reads the cached log evidence
|
||||
/// directly, so these are not on any production path.
|
||||
#[cfg(test)]
|
||||
impl MarginFactor {
|
||||
pub(crate) fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64) {
|
||||
self.propagate_with_alpha(vars, 1.0)
|
||||
}
|
||||
|
||||
fn log_evidence(&self, _vars: &VarStore) -> f64 {
|
||||
pub(crate) fn log_evidence(&self) -> f64 {
|
||||
self.log_evidence_cached.unwrap_or(0.0)
|
||||
}
|
||||
}
|
||||
@@ -146,7 +150,7 @@ mod tests {
|
||||
let diff = vars.alloc(Gaussian::from_ms(0.0, 6.0));
|
||||
let mut f = MarginFactor::new(diff, 5.0, 1.0);
|
||||
f.propagate(&mut vars);
|
||||
let logz = f.log_evidence(&vars);
|
||||
let logz = f.log_evidence();
|
||||
assert!((logz - (-3.062235327364623)).abs() < 1e-10);
|
||||
}
|
||||
|
||||
|
||||
+4
-72
@@ -20,6 +20,8 @@ pub struct VarStore {
|
||||
}
|
||||
|
||||
impl VarStore {
|
||||
/// Test-only: inference allocates its store through `ScratchArena`.
|
||||
#[cfg(test)]
|
||||
#[must_use]
|
||||
pub fn new() -> Self {
|
||||
Self::default()
|
||||
@@ -29,16 +31,13 @@ impl VarStore {
|
||||
self.marginals.clear();
|
||||
}
|
||||
|
||||
/// Test-only, as `new`.
|
||||
#[cfg(test)]
|
||||
#[must_use]
|
||||
pub fn len(&self) -> usize {
|
||||
self.marginals.len()
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn is_empty(&self) -> bool {
|
||||
self.marginals.is_empty()
|
||||
}
|
||||
|
||||
pub fn alloc(&mut self, init: Gaussian) -> VarId {
|
||||
let id = VarId(self.marginals.len() as u32);
|
||||
self.marginals.push(init);
|
||||
@@ -55,58 +54,7 @@ impl VarStore {
|
||||
}
|
||||
}
|
||||
|
||||
/// A factor in the EP graph.
|
||||
///
|
||||
/// Factors hold their own outgoing messages and propagate them by reading
|
||||
/// connected variable marginals from a `VarStore` and writing back updated
|
||||
/// marginals.
|
||||
pub trait Factor: Send + Sync {
|
||||
/// Update outgoing messages and write back to the var store.
|
||||
///
|
||||
/// Returns the max delta `(|Δmu|, |Δsigma|)` across writes this
|
||||
/// propagation. Used by the `Schedule` to detect convergence.
|
||||
fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64);
|
||||
|
||||
/// Optional log-evidence contribution. Default 0.0 (no contribution).
|
||||
fn log_evidence(&self, _vars: &VarStore) -> f64 {
|
||||
0.0
|
||||
}
|
||||
}
|
||||
|
||||
/// Enum dispatcher for the built-in factor types.
|
||||
///
|
||||
/// Using an enum instead of `Box<dyn Factor>` keeps factor data inline and
|
||||
/// avoids virtual-call overhead in the hot inference loop.
|
||||
#[derive(Debug)]
|
||||
pub enum BuiltinFactor {
|
||||
TeamSum(team_sum::TeamSumFactor),
|
||||
RankDiff(rank_diff::RankDiffFactor),
|
||||
Trunc(trunc::TruncFactor),
|
||||
Margin(margin::MarginFactor),
|
||||
}
|
||||
|
||||
impl Factor for BuiltinFactor {
|
||||
fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64) {
|
||||
match self {
|
||||
Self::TeamSum(f) => f.propagate(vars),
|
||||
Self::RankDiff(f) => f.propagate(vars),
|
||||
Self::Trunc(f) => f.propagate(vars),
|
||||
Self::Margin(f) => f.propagate(vars),
|
||||
}
|
||||
}
|
||||
|
||||
fn log_evidence(&self, vars: &VarStore) -> f64 {
|
||||
match self {
|
||||
Self::Trunc(f) => f.log_evidence(vars),
|
||||
Self::Margin(f) => f.log_evidence(vars),
|
||||
Self::TeamSum(_) | Self::RankDiff(_) => 0.0,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub mod margin;
|
||||
pub mod rank_diff;
|
||||
pub mod team_sum;
|
||||
pub mod trunc;
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -153,20 +101,4 @@ mod tests {
|
||||
assert_eq!(store.len(), 0);
|
||||
assert_eq!(store.marginals.capacity(), cap);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn builtin_factor_dispatches_to_margin() {
|
||||
use super::margin::MarginFactor;
|
||||
let mut vars = VarStore::new();
|
||||
let diff = vars.alloc(Gaussian::from_ms(0.0, 6.0));
|
||||
let mut f = BuiltinFactor::Margin(MarginFactor::new(diff, 5.0, 1.0));
|
||||
|
||||
f.propagate(&mut vars);
|
||||
|
||||
let result = vars.get(diff);
|
||||
assert!((result.mu() - 4.864864864864865).abs() < 1e-12);
|
||||
|
||||
let logz = f.log_evidence(&vars);
|
||||
assert!((logz - (-3.062235327364623)).abs() < 1e-10);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,95 +0,0 @@
|
||||
use crate::factor::{Factor, VarId, VarStore};
|
||||
|
||||
/// Maintains the constraint `diff = team_a - team_b` between three vars.
|
||||
///
|
||||
/// On each propagation:
|
||||
/// - Reads marginals at `team_a` and `team_b` (which already incorporate any
|
||||
/// incoming messages from neighboring factors).
|
||||
/// - Computes `new_diff = team_a - team_b` (variance addition; see `Gaussian::Sub`).
|
||||
/// - Writes the new marginal to `diff`.
|
||||
/// - Returns the delta against the previous diff value.
|
||||
///
|
||||
/// This factor does NOT store an outgoing message; the diff variable is
|
||||
/// effectively replaced on each propagation. The `TruncFactor` on the same diff
|
||||
/// var holds the EP-divide message that produces the cavity.
|
||||
#[derive(Debug)]
|
||||
pub struct RankDiffFactor {
|
||||
pub team_a: VarId,
|
||||
pub team_b: VarId,
|
||||
pub diff: VarId,
|
||||
}
|
||||
|
||||
impl Factor for RankDiffFactor {
|
||||
fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64) {
|
||||
let a = vars.get(self.team_a);
|
||||
let b = vars.get(self.team_b);
|
||||
let new_diff = a - b;
|
||||
let old = vars.get(self.diff);
|
||||
vars.set(self.diff, new_diff);
|
||||
old.delta(new_diff)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::{N_INF, gaussian::Gaussian};
|
||||
|
||||
#[test]
|
||||
fn diff_of_two_known_gaussians() {
|
||||
let mut vars = VarStore::new();
|
||||
let team_a = vars.alloc(Gaussian::from_ms(25.0, 3.0));
|
||||
let team_b = vars.alloc(Gaussian::from_ms(20.0, 4.0));
|
||||
let diff = vars.alloc(N_INF);
|
||||
|
||||
let mut f = RankDiffFactor {
|
||||
team_a,
|
||||
team_b,
|
||||
diff,
|
||||
};
|
||||
f.propagate(&mut vars);
|
||||
|
||||
let result = vars.get(diff);
|
||||
// mu = 25 - 20 = 5; var = 9 + 16 = 25; sigma = 5
|
||||
assert!((result.mu() - 5.0).abs() < 1e-12);
|
||||
assert!((result.sigma() - 5.0).abs() < 1e-12);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn delta_zero_on_repeat() {
|
||||
let mut vars = VarStore::new();
|
||||
let team_a = vars.alloc(Gaussian::from_ms(10.0, 2.0));
|
||||
let team_b = vars.alloc(Gaussian::from_ms(8.0, 1.0));
|
||||
let diff = vars.alloc(N_INF);
|
||||
|
||||
let mut f = RankDiffFactor {
|
||||
team_a,
|
||||
team_b,
|
||||
diff,
|
||||
};
|
||||
f.propagate(&mut vars);
|
||||
let (dmu, dsig) = f.propagate(&mut vars);
|
||||
assert!(dmu < 1e-12);
|
||||
assert!(dsig < 1e-12);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn delta_reflects_team_change() {
|
||||
let mut vars = VarStore::new();
|
||||
let team_a = vars.alloc(Gaussian::from_ms(10.0, 1.0));
|
||||
let team_b = vars.alloc(Gaussian::from_ms(0.0, 1.0));
|
||||
let diff = vars.alloc(N_INF);
|
||||
|
||||
let mut f = RankDiffFactor {
|
||||
team_a,
|
||||
team_b,
|
||||
diff,
|
||||
};
|
||||
f.propagate(&mut vars);
|
||||
|
||||
// change team_a, repropagate; delta should be positive
|
||||
vars.set(team_a, Gaussian::from_ms(15.0, 1.0));
|
||||
let (dmu, _dsig) = f.propagate(&mut vars);
|
||||
assert!(dmu > 4.0, "expected ~5 delta, got {}", dmu);
|
||||
}
|
||||
}
|
||||
@@ -1,98 +0,0 @@
|
||||
use crate::{
|
||||
N00,
|
||||
factor::{Factor, VarId, VarStore},
|
||||
gaussian::Gaussian,
|
||||
};
|
||||
|
||||
/// Computes the weighted sum of player performances into a team-perf var.
|
||||
///
|
||||
/// Inputs are pre-computed player performance Gaussians (i.e., rating priors
|
||||
/// already with beta² noise added via `Rating::performance()`). The factor
|
||||
/// runs once per game and writes the weighted sum to the output var.
|
||||
#[derive(Debug)]
|
||||
pub struct TeamSumFactor {
|
||||
pub inputs: Vec<(Gaussian, f64)>,
|
||||
pub out: VarId,
|
||||
}
|
||||
|
||||
impl Factor for TeamSumFactor {
|
||||
fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64) {
|
||||
let perf = self.inputs.iter().fold(N00, |acc, (g, w)| acc + (*g * *w));
|
||||
let old = vars.get(self.out);
|
||||
vars.set(self.out, perf);
|
||||
old.delta(perf)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::N_INF;
|
||||
|
||||
#[test]
|
||||
fn single_player_unit_weight() {
|
||||
let mut vars = VarStore::new();
|
||||
let out = vars.alloc(N_INF);
|
||||
let g = Gaussian::from_ms(25.0, 5.0);
|
||||
let mut f = TeamSumFactor {
|
||||
inputs: vec![(g, 1.0)],
|
||||
out,
|
||||
};
|
||||
|
||||
f.propagate(&mut vars);
|
||||
let result = vars.get(out);
|
||||
assert!((result.mu() - 25.0).abs() < 1e-12);
|
||||
assert!((result.sigma() - 5.0).abs() < 1e-12);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn two_players_summed() {
|
||||
let mut vars = VarStore::new();
|
||||
let out = vars.alloc(N_INF);
|
||||
let g1 = Gaussian::from_ms(20.0, 3.0);
|
||||
let g2 = Gaussian::from_ms(30.0, 4.0);
|
||||
let mut f = TeamSumFactor {
|
||||
inputs: vec![(g1, 1.0), (g2, 1.0)],
|
||||
out,
|
||||
};
|
||||
|
||||
f.propagate(&mut vars);
|
||||
let result = vars.get(out);
|
||||
// sum: mu = 20 + 30 = 50, var = 9 + 16 = 25, sigma = 5
|
||||
assert!((result.mu() - 50.0).abs() < 1e-12);
|
||||
assert!((result.sigma() - 5.0).abs() < 1e-12);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn weighted_inputs() {
|
||||
let mut vars = VarStore::new();
|
||||
let out = vars.alloc(N_INF);
|
||||
let g = Gaussian::from_ms(10.0, 2.0);
|
||||
let mut f = TeamSumFactor {
|
||||
inputs: vec![(g, 2.0)],
|
||||
out,
|
||||
};
|
||||
|
||||
f.propagate(&mut vars);
|
||||
let result = vars.get(out);
|
||||
// g * 2.0: mu = 10*2 = 20, sigma = 2*2 = 4
|
||||
assert!((result.mu() - 20.0).abs() < 1e-12);
|
||||
assert!((result.sigma() - 4.0).abs() < 1e-12);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn delta_is_zero_on_repeat_propagate() {
|
||||
let mut vars = VarStore::new();
|
||||
let out = vars.alloc(N_INF);
|
||||
let g = Gaussian::from_ms(5.0, 1.0);
|
||||
let mut f = TeamSumFactor {
|
||||
inputs: vec![(g, 1.0)],
|
||||
out,
|
||||
};
|
||||
|
||||
f.propagate(&mut vars);
|
||||
let (dmu, dsig) = f.propagate(&mut vars);
|
||||
assert!(dmu < 1e-12, "expected ~0 delta on repeat, got {}", dmu);
|
||||
assert!(dsig < 1e-12);
|
||||
}
|
||||
}
|
||||
+7
-7
@@ -1,6 +1,6 @@
|
||||
use crate::{
|
||||
N_INF, approx,
|
||||
factor::{Factor, VarId, VarStore},
|
||||
factor::{VarId, VarStore},
|
||||
gaussian::Gaussian,
|
||||
ln_interval, ln_sf,
|
||||
};
|
||||
@@ -63,14 +63,14 @@ impl TruncFactor {
|
||||
}
|
||||
}
|
||||
|
||||
impl Factor for TruncFactor {
|
||||
fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64) {
|
||||
/// Undamped wrappers, used by this module's tests. Inference drives these
|
||||
/// factors through `propagate_with_alpha` and reads the cached log evidence
|
||||
/// directly, so these are not on any production path.
|
||||
#[cfg(test)]
|
||||
impl TruncFactor {
|
||||
pub(crate) fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64) {
|
||||
self.propagate_with_alpha(vars, 1.0)
|
||||
}
|
||||
|
||||
fn log_evidence(&self, _vars: &VarStore) -> f64 {
|
||||
self.log_evidence_cached.unwrap_or(0.0)
|
||||
}
|
||||
}
|
||||
|
||||
/// `ln P(diff > margin)` for a win, `ln P(|diff| < margin)` for a tie.
|
||||
|
||||
Reference in New Issue
Block a user