refactor!: close the remaining API gaps from #21

Three unrelated small defects, all requiring signature changes:

- `Game::one_v_one` hardcoded `GameOptions::default()`, so a 1v1 could
  never set `p_draw` or convergence options — and a drawn 1v1 was
  therefore unreachable through it, since the default `p_draw` is zero.
  It now takes `&GameOptions` like every other constructor.

- `Observer::on_batch_processed` was declared on the trait and never
  called from anywhere: implementors wired up a callback that could not
  fire. It is now called after each slice sweep, and renamed
  `on_slice_processed` to match the vocabulary the codebase adopted in
  T2 — the unit of work is a `TimeSlice`, not a batch. A slice is swept
  once travelling backward and once forward, so a multi-slice history
  fires it twice per slice per iteration; the doc comment says so.

- `pub mod factors` sat beside `pub(crate) mod factor`, two module paths
  differing by one character with only one of them importable. The
  public facade is now `graph`.

Tests cover each as a behaviour rather than a compile check: a drawn 1v1
succeeds only when p_draw is supplied, and the observer tests fail if
any callback stops firing.

BREAKING CHANGE: `Game::one_v_one` takes a fourth `&GameOptions`
argument; `Observer::on_batch_processed` is renamed
`on_slice_processed`; the `factors` module is renamed `graph`.

Closes #21

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
This commit is contained in:
2026-09-07 14:57:39 +02:00
co-authored by Claude Opus 5
parent bb2a845882
commit 507894dae7
8 changed files with 190 additions and 14 deletions
+11 -9
View File
@@ -532,18 +532,20 @@ impl<T: Time, D: Drift<T>> Game<'_, T, D> {
))
}
/// Convenience wrapper over [`Game::ranked`] for two single-player teams.
///
/// # Errors
///
/// Delegates to [`Game::ranked`] with default options, so it returns the
/// same errors — in practice `WrongOutcomeKind` for a non-ranked outcome,
/// or `TieWithoutDrawProbability` for a draw, since the default `p_draw`
/// applies rather than one you chose.
/// Delegates to [`Game::ranked`], so it returns the same errors — in
/// practice `WrongOutcomeKind` for a non-ranked outcome, or
/// `TieWithoutDrawProbability` for a draw when `options.p_draw` is zero.
pub fn one_v_one(
a: &Rating<T, D>,
b: &Rating<T, D>,
outcome: crate::Outcome,
options: &GameOptions,
) -> Result<(Gaussian, Gaussian), crate::InferenceError> {
let game = Self::ranked(&[&[*a], &[*b]], outcome, &GameOptions::default())?;
let game = Self::ranked(&[&[*a], &[*b]], outcome, options)?;
let post = game.posteriors();
Ok((post[0][0], post[1][0]))
}
@@ -563,11 +565,11 @@ impl<T: Time, D: Drift<T>> Game<'_, T, D> {
}
#[doc(hidden)]
pub fn custom<S: crate::factors::Schedule>(
factors: &mut [crate::factors::BuiltinFactor],
vars: &mut crate::factors::VarStore,
pub fn custom<S: crate::graph::Schedule>(
factors: &mut [crate::graph::BuiltinFactor],
vars: &mut crate::graph::VarStore,
schedule: &S,
) -> crate::factors::ScheduleReport {
) -> crate::graph::ScheduleReport {
schedule.run(factors, vars)
}
}
+4
View File
@@ -1,5 +1,9 @@
//! Factor-graph public API.
//!
//! Named `graph` rather than `factors` because the private implementation
//! module beside it is `factor`: two module paths differing by one character,
//! one public and one not, was a standing invitation to import the wrong one.
//!
//! The factor types, `VarStore` and the `Schedule` trait are public so custom
//! schedules can be written against them.
//!
+15
View File
@@ -261,6 +261,11 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
let old = self.time_slices[j].posteriors();
self.time_slices[j].new_backward_info(&self.agents);
self.observer.on_slice_processed(
&self.time_slices[j].time,
j,
self.time_slices[j].events.len(),
);
let new = self.time_slices[j].posteriors();
@@ -280,6 +285,11 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
let old = self.time_slices[j].posteriors();
self.time_slices[j].new_forward_info(&self.agents);
self.observer.on_slice_processed(
&self.time_slices[j].time,
j,
self.time_slices[j].events.len(),
);
let new = self.time_slices[j].posteriors();
@@ -292,6 +302,11 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
let old = self.time_slices[0].posteriors();
self.time_slices[0].iteration(0, &self.agents);
self.observer.on_slice_processed(
&self.time_slices[0].time,
0,
self.time_slices[0].events.len(),
);
let new = self.time_slices[0].posteriors();
+1 -1
View File
@@ -118,9 +118,9 @@ mod error;
mod event;
mod event_builder;
pub(crate) mod factor;
pub mod factors;
mod game;
pub mod gaussian;
pub mod graph;
mod history;
mod key_table;
mod matrix;
+8 -2
View File
@@ -14,8 +14,13 @@ pub trait Observer<T: Time>: Send + Sync {
/// Called after each convergence iteration across the whole history.
fn on_iteration_end(&self, _iter: usize, _max_step: (f64, f64)) {}
/// Called after each time slice is processed within an iteration.
fn on_batch_processed(&self, _time: &T, _slice_idx: usize, _n_events: usize) {}
/// Called after each time slice is swept within an iteration.
///
/// A convergence iteration sweeps every slice twice — once travelling
/// backward through the history and once forward — so a multi-slice
/// history fires this twice per slice per iteration. A single-slice
/// history is swept once and fires once.
fn on_slice_processed(&self, _time: &T, _slice_idx: usize, _n_events: usize) {}
/// Called once when convergence completes (or max iters is reached).
fn on_converged(&self, _iters: usize, _final_step: (f64, f64), _converged: bool) {}
@@ -35,6 +40,7 @@ mod tests {
fn null_observer_compiles_for_i64() {
let o = NullObserver;
<NullObserver as Observer<i64>>::on_iteration_end(&o, 1, (0.0, 0.0));
<NullObserver as Observer<i64>>::on_slice_processed(&o, &7, 0, 3);
<NullObserver as Observer<i64>>::on_converged(&o, 5, (1e-6, 1e-6), true);
}