Compare commits
19 Commits
v0.1.1
..
68be7ab5b7
| Author | SHA1 | Date | |
|---|---|---|---|
| 68be7ab5b7 | |||
| 824b7f50b0 | |||
| 872f91797d | |||
| 6e453b6845 | |||
| 965ea7ed3c | |||
| dbce69f350 | |||
| 0705986929 | |||
| aacaa60baa | |||
| fcfe0ffe37 | |||
| 0fa4e7d277 | |||
| 0dd7dab266 | |||
| 43cc6d82f9 | |||
| 48a6049dc6 | |||
| 1445c08896 | |||
| f6a83e4dc6 | |||
| 68b589b965 | |||
| 7481c31ad8 | |||
| a69a3004b2 | |||
| dbaad0e7d2 |
+7
-141
@@ -2,149 +2,13 @@
|
||||
|
||||
All notable changes to this project will be documented in this file.
|
||||
|
||||
## Unreleased — T3 concurrency
|
||||
## 0.1.1 - 2026-04-27
|
||||
|
||||
Adds rayon-backed parallel paths per Section 6 of
|
||||
`docs/superpowers/specs/2026-04-23-trueskill-engine-redesign-design.md`.
|
||||
### Other (unconventional)
|
||||
|
||||
### Breaking
|
||||
|
||||
- `Send + Sync` bounds added to public traits: `Time`, `Drift<T>`,
|
||||
`Observer<T>`, `Factor`, `Schedule`. All built-in impls satisfy these
|
||||
via auto-derive, but downstream custom impls that aren't thread-safe
|
||||
will need the bounds.
|
||||
|
||||
### New
|
||||
|
||||
- Opt-in `rayon` cargo feature. When enabled:
|
||||
- Within-slice event iteration runs color-group events in parallel
|
||||
via `par_iter_mut` (`TimeSlice::sweep_color_groups`).
|
||||
- `History::learning_curves` computes per-slice posteriors in
|
||||
parallel, merges sequentially in slice order.
|
||||
- `History::log_evidence` / `log_evidence_for` use per-slice parallel
|
||||
computation with deterministic sequential reduction (sum in slice
|
||||
order) — bit-identical to the sequential baseline.
|
||||
- `ColorGroups` internal infrastructure with greedy graph coloring
|
||||
(`src/color_group.rs`). Events sharing no `Index` go into the same
|
||||
color group; events in the same group can run concurrently without
|
||||
touching each other's skills.
|
||||
- `tests/determinism.rs` asserts bit-identical posteriors across
|
||||
`RAYON_NUM_THREADS={1, 2, 4, 8}`.
|
||||
- `benches/history_converge.rs` measures end-to-end convergence on
|
||||
three workload shapes.
|
||||
|
||||
### Performance notes
|
||||
|
||||
- Default build (no rayon): `Batch::iteration` 23.23 µs — no regression
|
||||
vs T2.
|
||||
- With `--features rayon`:
|
||||
- 500 events / 100 competitors / 10 per slice: 1.0× speedup.
|
||||
- 2000 events / 200 competitors / 20 per slice: 1.0× speedup.
|
||||
- 5000 events in one slice / 50k competitors: **1.3× speedup.**
|
||||
- The spec targeted >2× speedup on 8-core offline converge. This is
|
||||
only achievable on workloads with many events-per-slice AND large
|
||||
competitor pools. **Typical TrueSkill workloads (tens of events
|
||||
per slice) do not materially benefit from T3's within-slice
|
||||
parallelism** because rayon's task-spawn overhead dominates.
|
||||
- Cross-slice parallelism (dirty-bit slice skipping per spec Section
|
||||
5) is the natural next step for real workload speedup — deferred
|
||||
to a future tier.
|
||||
|
||||
### Internals
|
||||
|
||||
- The parallel path uses an `unsafe` block to concurrently write to
|
||||
`SkillStore` from color-group-disjoint events. Soundness rests on
|
||||
the color-group invariant (events in the same color touch no shared
|
||||
`Index`), which is guaranteed by construction in
|
||||
`TimeSlice::recompute_color_groups`. Sequential path unchanged.
|
||||
- `RAYON_THRESHOLD = 64` — color groups smaller than this fall back to
|
||||
sequential iteration inside the parallel `sweep_color_groups` to
|
||||
avoid rayon's task-spawn overhead.
|
||||
- Thread-local `ScratchArena` per rayon worker thread.
|
||||
|
||||
## Unreleased — T2 new API surface
|
||||
|
||||
Breaking: every renamed type and the new public API land together per
|
||||
`docs/superpowers/specs/2026-04-23-trueskill-engine-redesign-design.md`
|
||||
Section 7 "T2".
|
||||
|
||||
### Breaking renames
|
||||
|
||||
- `Batch` → `TimeSlice`
|
||||
- `Player` → `Rating` (and the `.player` field on `Competitor` is now `.rating`)
|
||||
- `Agent` → `Competitor`
|
||||
- `IndexMap` → `KeyTable`
|
||||
- `History` field `.batches` → `.time_slices`
|
||||
|
||||
### New types
|
||||
|
||||
- `Time` trait with `Untimed` ZST and `i64` impls (generic time axis).
|
||||
- `Drift<T: Time>` — generified from the old `Drift` trait.
|
||||
- `Event<T, K>`, `Team<K>`, `Member<K>` — typed bulk-ingest event shape.
|
||||
- `Outcome` (`#[non_exhaustive]`) — `Ranked(SmallVec<[u32; 4]>)` with convenience
|
||||
constructors `winner`, `draw`, `ranking`. `Scored` lands in T4.
|
||||
- `Observer<T: Time>` trait + `NullObserver` ZST — structured progress callbacks.
|
||||
- `ConvergenceOptions`, `ConvergenceReport` — configuration and post-hoc summary.
|
||||
- `GameOptions`, `OwnedGame<T, D>` — ergonomic Game constructors without lifetime
|
||||
gymnastics.
|
||||
- `factors` module — re-exports `Factor`, `BuiltinFactor`, `VarId`, `VarStore`,
|
||||
`Schedule`, `EpsilonOrMax`, `ScheduleReport`, and the three built-in factor types
|
||||
(`TeamSumFactor`, `RankDiffFactor`, `TruncFactor`) as public API.
|
||||
|
||||
### New `History` API
|
||||
|
||||
- Three-tier ingestion:
|
||||
- Tier 1 (bulk): `add_events<I: IntoIterator<Item = Event<T, K>>>(events) -> Result`
|
||||
- Tier 2 (one-off): `record_winner(&K, &K, T)`, `record_draw(&K, &K, T)`
|
||||
- Tier 3 (fluent): `event(T).team([...]).weights([...]).ranking([...]).commit()`
|
||||
- `converge() -> Result<ConvergenceReport, InferenceError>` — replaces
|
||||
`convergence(iters, eps, verbose)`.
|
||||
- `current_skill(&K)`, `learning_curve(&K)`, `learning_curves()` (now keyed on `K`).
|
||||
- `log_evidence()` zero-arg, `log_evidence_for(&[&K])`.
|
||||
- `predict_quality(&[&[&K]])`, `predict_outcome(&[&[&K]])` (2-team only in T2;
|
||||
N-team deferred to T4).
|
||||
- `intern(&Q)` / `lookup(&Q)` expose the internal `KeyTable<K>` for power users.
|
||||
- `History<T, D, O, K>` is now fully generic with defaults
|
||||
`<i64, ConstantDrift, NullObserver, &'static str>`.
|
||||
|
||||
### New `Game` API
|
||||
|
||||
- `Game::ranked(&[&[Rating]], Outcome, &GameOptions) -> Result<OwnedGame, _>`.
|
||||
- `Game::one_v_one(&Rating, &Rating, Outcome) -> Result<(Gaussian, Gaussian), _>`.
|
||||
- `Game::free_for_all(&[&Rating], Outcome, &GameOptions) -> Result<OwnedGame, _>`.
|
||||
- `Game::custom(...)` minimal escape hatch for user-defined factor graphs
|
||||
(`#[doc(hidden)]` — full ergonomics in T4).
|
||||
- `Game::log_evidence()` and `OwnedGame::log_evidence()` accessors.
|
||||
|
||||
### Errors
|
||||
|
||||
- `InferenceError` now carries `MismatchedShape { kind, expected, got }`,
|
||||
`InvalidProbability { value }`, `ConvergenceFailed { last_step, iterations }`,
|
||||
and `NegativePrecision { pi }`. Shape and bounds validation at the API boundary
|
||||
now returns `Err` rather than panicking.
|
||||
|
||||
### Removed (breaking)
|
||||
|
||||
- `History::convergence(iters, eps, verbose)` — use `converge()`.
|
||||
- `HistoryBuilder::gamma(f64)` — use `.drift(ConstantDrift(g))`.
|
||||
- `HistoryBuilder::time(bool)` and `History.time: bool` — use the `Time` type parameter.
|
||||
- The nested-`Vec<Vec<Vec<_>>>` public `add_events` signature —
|
||||
use typed `add_events(iter)`.
|
||||
- `learning_curves_by_index()` — use `learning_curves()`.
|
||||
|
||||
### Performance
|
||||
|
||||
`Batch::iteration` bench: **21.36 µs** (T1 was 22.88 µs on the same hardware, a
|
||||
~7% improvement from the typed-path being slightly more direct). Gaussian
|
||||
operations unchanged.
|
||||
|
||||
### Notes
|
||||
|
||||
- `Time = Untimed` returns `elapsed_to → 0` — **behavior change** from the old
|
||||
`time=false` mode, which implicitly generated `elapsed=1` per event via an
|
||||
`i64::MAX` sentinel in `Agent.last_time`. Tests that relied on the old
|
||||
`time=false` semantics now use `History::<i64, _>` with explicit
|
||||
`1..=n` timestamps.
|
||||
- T0 + T1 + T2: engine redesign through new API surface (#1)
|
||||
- T3: rayon-backed concurrency (opt-in) (#2)
|
||||
- T4 (MarginFactor): scored outcomes via Gaussian-margin EP evidence
|
||||
|
||||
## 0.1.0 - 2026-04-23
|
||||
|
||||
@@ -156,6 +20,8 @@ operations unchanged.
|
||||
|
||||
- chore: added cliff.toml, release.toml and rustfmt.toml
|
||||
- chore: clean up
|
||||
- chore: make cargo release add CHANGELOG.md before commit
|
||||
- chore: do not publish
|
||||
|
||||
### Other (unconventional)
|
||||
|
||||
|
||||
+3
-3
@@ -1,7 +1,7 @@
|
||||
use criterion::{Criterion, criterion_group, criterion_main};
|
||||
use trueskill_tt::{
|
||||
BETA, Competitor, EventKind, GAMMA, KeyTable, MU, P_DRAW, Rating, SIGMA, TimeSlice,
|
||||
drift::ConstantDrift, gaussian::Gaussian, storage::CompetitorStore,
|
||||
BETA, Competitor, ConvergenceOptions, EventKind, GAMMA, KeyTable, MU, P_DRAW, Rating, SIGMA,
|
||||
TimeSlice, drift::ConstantDrift, gaussian::Gaussian, storage::CompetitorStore,
|
||||
};
|
||||
|
||||
fn criterion_benchmark(criterion: &mut Criterion) {
|
||||
@@ -35,7 +35,7 @@ fn criterion_benchmark(criterion: &mut Criterion) {
|
||||
|
||||
let kinds = vec![EventKind::Ranked; composition.len()];
|
||||
|
||||
let mut time_slice = TimeSlice::new(1, P_DRAW);
|
||||
let mut time_slice = TimeSlice::new(1, P_DRAW, ConvergenceOptions::default());
|
||||
time_slice.add_events(composition, results, weights, kinds, &agents);
|
||||
|
||||
criterion.bench_function("Batch::iteration", |b| {
|
||||
|
||||
@@ -51,6 +51,7 @@ fn build_history_1v1(
|
||||
.convergence(ConvergenceOptions {
|
||||
max_iter: 30,
|
||||
epsilon: 1e-6,
|
||||
alpha: 1.0,
|
||||
})
|
||||
.build();
|
||||
|
||||
|
||||
@@ -49,7 +49,7 @@ A Gaussian `N(m, σ)` constructed via `Gaussian::from_ms(m, σ)`. Multiplication
|
||||
**Concrete numerical check for tests:** With cavity `N(0, 6)` and observation `m_obs=5, σ=1`:
|
||||
- `D_cav.pi = 1/36 ≈ 0.027778`, `D_cav.tau = 0`.
|
||||
- New marginal: `pi = 0.027778 + 1 = 1.027778`, `tau = 0 + 5 = 5`. So `mu = 5 / 1.027778 ≈ 4.864865`, `sigma = 1/sqrt(1.027778) ≈ 0.986394`.
|
||||
- `Z_cav = pdf(5, 0, sqrt(36 + 1)) = pdf(5, 0, sqrt(37)) ≈ 0.046827`. So `log_evidence ≈ -3.0613`.
|
||||
- `Z_cav = pdf(5, 0, sqrt(36 + 1)) = pdf(5, 0, sqrt(37)) ≈ 0.04678`. So `log_evidence ≈ -3.0622`.
|
||||
|
||||
---
|
||||
|
||||
@@ -182,7 +182,7 @@ mod tests {
|
||||
|
||||
f.propagate(&mut vars);
|
||||
let z = f.evidence_cached.unwrap();
|
||||
// pdf(5, 0, sqrt(37)) ≈ 0.046827
|
||||
// pdf(5, 0, sqrt(37)) ≈ 0.04678
|
||||
assert!((z - 0.04682752233851171).abs() < 1e-10);
|
||||
|
||||
// Subsequent propagations don't change it.
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,593 @@
|
||||
# History → TimeSlice ConvergenceOptions Plumbing Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Thread `ConvergenceOptions` from `History` through `TimeSlice` to the three `Game::*_with_arena` callsites in `time_slice.rs`, so users who set `HistoryBuilder::convergence(opts)` actually get those options applied to within-game inference (including Damped's `alpha`).
|
||||
|
||||
**Architecture:** `TimeSlice<T>` gains a `convergence: ConvergenceOptions` field set at construction. `History::add_events_with_prior` passes `self.convergence`. The three `Game::*_with_arena` callsites in `time_slice.rs` swap their hardcoded `ConvergenceOptions::default()` for the propagated value. The pre-existing `TimeSlice::convergence` method is renamed to `iterate_to_convergence` to disambiguate from the new field. No new public API on `History` or `HistoryBuilder` — `convergence(opts)` already exists and works.
|
||||
|
||||
**Tech Stack:** Rust 2024, `cargo +nightly fmt`, `cargo clippy`, `cargo test --lib`.
|
||||
|
||||
---
|
||||
|
||||
## Spec reference
|
||||
|
||||
`docs/superpowers/specs/2026-05-08-history-convergence-plumbing-design.md`
|
||||
|
||||
## Pre-flight context for the implementer
|
||||
|
||||
- `HistoryBuilder::convergence(opts)` already exists at `src/history.rs:91`. `History` already stores `convergence: ConvergenceOptions` at `src/history.rs:166`. `History::converge()` already reads `self.convergence.{epsilon, max_iter}` at `src/history.rs:437-447` for the OUTER cross-history loop.
|
||||
- `TimeSlice<T>` is at `src/time_slice.rs:172-180`. Currently has fields `events`, `skills`, `time`, `p_draw`, `arena`, `color_groups`. No convergence field yet.
|
||||
- `TimeSlice::new(time, p_draw)` at `src/time_slice.rs:183-192` is `pub`. Five test callsites use it with `(0i64, 0.0)`. One production callsite in `History::add_events_with_prior` at `src/history.rs:597` uses `(t, self.p_draw)`.
|
||||
- Three callsites in `time_slice.rs` call `Game::*_with_arena` with hardcoded `crate::ConvergenceOptions::default()`:
|
||||
- `Event::iteration_direct` at `src/time_slice.rs:131-169` — does NOT have `&self` access to a TimeSlice. Currently takes `(skills, agents, p_draw, arena)`. Needs to gain a `convergence` parameter.
|
||||
- `TimeSlice::iteration` at `src/time_slice.rs:322-363` — has `&mut self`, so reads `self.convergence` directly.
|
||||
- `TimeSlice::log_evidence` at `src/time_slice.rs:505-540` — has `&self`, so reads `self.convergence` directly.
|
||||
- The rayon path in `sweep_color_groups` at `src/time_slice.rs:376-423` uses a `move` closure capturing `p_draw` by value. The same pattern applies to `convergence` (it's `Copy`, so captures cleanly).
|
||||
- `TimeSlice::convergence` (the **method** at `src/time_slice.rs:447`) shares its name with the new field. Rust technically allows this (different namespaces), but it's a readability hazard — must be renamed. The method is called from 4 test sites in `time_slice.rs` (lines 693, 755, 817, 851). It is NOT called from `history.rs`.
|
||||
- `ConvergenceOptions` is `Copy + Clone + Debug`. Pass by value everywhere.
|
||||
|
||||
## File map
|
||||
|
||||
| File | Why touched |
|
||||
|---|---|
|
||||
| `src/time_slice.rs` | TimeSlice gains `convergence` field, `new` signature change, rename `convergence` method, three callsites read `self.convergence`, `Event::iteration_direct` gains parameter, rayon closure captures it |
|
||||
| `src/history.rs` | `add_events_with_prior` passes `self.convergence` to `TimeSlice::new`; two integration tests added; alpha doc-comment update happens in `convergence.rs` not here |
|
||||
| `src/convergence.rs` | One-sentence addition to `alpha` doc comment clarifying within-game-only scope |
|
||||
|
||||
---
|
||||
|
||||
### Task 1: TimeSlice gains `convergence` field; signature/rename land atomically
|
||||
|
||||
This task does five things atomically — they cannot land separately because intermediate states won't compile:
|
||||
|
||||
1. Add `pub(crate) convergence: ConvergenceOptions` field to `TimeSlice<T>`.
|
||||
2. Change `TimeSlice::new` signature to take `convergence: ConvergenceOptions` as the third parameter.
|
||||
3. Update the production callsite in `History::add_events_with_prior` (`src/history.rs:597`) to pass `self.convergence`.
|
||||
4. Update the five test callsites in `src/time_slice.rs` (lines 646, 723, 803, 901 — the four with `TimeSlice::new(0i64, 0.0)`, plus the one inside the test module's `iterate_through_color_groups` test if it exists; locate via `grep -n "TimeSlice::new" src/time_slice.rs`).
|
||||
5. Rename the existing `pub(crate) fn convergence` method (at `src/time_slice.rs:447`) to `iterate_to_convergence`. Update its 4 in-file call sites.
|
||||
|
||||
After this task the convergence field is wired but **unused** by inference (Task 2 makes the three Game callsites read it). All existing tests must pass bit-equal because the propagated value still equals `ConvergenceOptions::default()` end-to-end.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/time_slice.rs`
|
||||
- Modify: `src/history.rs:597`
|
||||
|
||||
- [ ] **Step 1: Locate all `TimeSlice::new` and `convergence`-method callsites**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
grep -n "TimeSlice::new\|\.convergence(" src/time_slice.rs src/history.rs
|
||||
```
|
||||
|
||||
Expected: 1 production callsite of `TimeSlice::new` in `history.rs`, 5 test callsites in `time_slice.rs`, and 4 method-style `.convergence(` calls in `time_slice.rs` test module. (No `.convergence(` calls in `history.rs` — those are field accesses.)
|
||||
|
||||
Save the line numbers — you'll need them in Step 4 and Step 6.
|
||||
|
||||
- [ ] **Step 2: Add the `convergence` field to `TimeSlice<T>`**
|
||||
|
||||
In `src/time_slice.rs`, modify the `TimeSlice<T>` struct (currently at `src/time_slice.rs:172-180`):
|
||||
|
||||
```rust
|
||||
#[derive(Debug)]
|
||||
pub struct TimeSlice<T: Time = i64> {
|
||||
pub(crate) events: Vec<Event>,
|
||||
pub(crate) skills: SkillStore,
|
||||
pub(crate) time: T,
|
||||
p_draw: f64,
|
||||
pub(crate) convergence: crate::ConvergenceOptions,
|
||||
arena: ScratchArena,
|
||||
pub(crate) color_groups: ColorGroups,
|
||||
}
|
||||
```
|
||||
|
||||
Code won't compile until Step 3.
|
||||
|
||||
- [ ] **Step 3: Change `TimeSlice::new` signature**
|
||||
|
||||
In `src/time_slice.rs`, replace the existing `pub fn new` (currently at `src/time_slice.rs:183-192`) with:
|
||||
|
||||
```rust
|
||||
pub fn new(time: T, p_draw: f64, convergence: crate::ConvergenceOptions) -> Self {
|
||||
Self {
|
||||
events: Vec::new(),
|
||||
skills: SkillStore::new(),
|
||||
time,
|
||||
p_draw,
|
||||
convergence,
|
||||
arena: ScratchArena::new(),
|
||||
color_groups: ColorGroups::new(),
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Update the production callsite in `history.rs`**
|
||||
|
||||
In `src/history.rs:597`, replace:
|
||||
|
||||
```rust
|
||||
let mut time_slice = TimeSlice::new(t, self.p_draw);
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```rust
|
||||
let mut time_slice = TimeSlice::new(t, self.p_draw, self.convergence);
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Update test callsites of `TimeSlice::new`**
|
||||
|
||||
Run `cargo build --tests` to surface every remaining compile error. Each error is a `TimeSlice::new(time, p_draw)` callsite missing the third argument. The fix: add `crate::ConvergenceOptions::default(),` (inside `src/time_slice.rs` test modules use the path relative to where `ConvergenceOptions` is in scope — if it's not imported in that test mod, add `use crate::ConvergenceOptions;` at the top of the mod and pass `ConvergenceOptions::default()`).
|
||||
|
||||
Example transformation. Before:
|
||||
|
||||
```rust
|
||||
let mut time_slice = TimeSlice::new(0i64, 0.0);
|
||||
```
|
||||
|
||||
After:
|
||||
|
||||
```rust
|
||||
let mut time_slice = TimeSlice::new(0i64, 0.0, crate::ConvergenceOptions::default());
|
||||
```
|
||||
|
||||
Apply to all 5 test callsites identified in Step 1. Repeat `cargo build --tests` until it succeeds.
|
||||
|
||||
- [ ] **Step 6: Rename the `convergence` method to `iterate_to_convergence`**
|
||||
|
||||
In `src/time_slice.rs`, find the method definition at `src/time_slice.rs:447`:
|
||||
|
||||
```rust
|
||||
pub(crate) fn convergence<D: Drift<T>>(&mut self, agents: &CompetitorStore<T, D>) -> usize {
|
||||
```
|
||||
|
||||
Rename to:
|
||||
|
||||
```rust
|
||||
pub(crate) fn iterate_to_convergence<D: Drift<T>>(&mut self, agents: &CompetitorStore<T, D>) -> usize {
|
||||
```
|
||||
|
||||
Then update the 4 call sites (located in Step 1 — `time_slice.rs:693, 755, 817, 851` or wherever your grep found them). At each site, replace `time_slice.convergence(&agents)` with `time_slice.iterate_to_convergence(&agents)`.
|
||||
|
||||
- [ ] **Step 7: Build and run the full test suite**
|
||||
|
||||
Run: `cargo build && cargo test --lib`
|
||||
|
||||
Expected: all 98 lib tests pass. Bit-equal goldens — the convergence field is wired but the three inference callsites still hardcode `ConvergenceOptions::default()` (Task 2 changes that), and the propagated default equals what was hardcoded before, so behavior is identical.
|
||||
|
||||
If any test fails: investigate. The most likely cause is a missed `TimeSlice::new` callsite or a `.convergence(` call site that needs renaming.
|
||||
|
||||
- [ ] **Step 8: Run integration tests**
|
||||
|
||||
Run: `cargo test`
|
||||
|
||||
Expected: all 27 integration tests still pass.
|
||||
|
||||
- [ ] **Step 9: Format and lint**
|
||||
|
||||
Run: `cargo +nightly fmt && cargo clippy --all-targets -- -D warnings`
|
||||
|
||||
Expected: no diff, no warnings.
|
||||
|
||||
- [ ] **Step 10: Commit**
|
||||
|
||||
```bash
|
||||
git add src/time_slice.rs src/history.rs
|
||||
git commit -m "$(cat <<'EOF'
|
||||
refactor(time_slice): add convergence field, rename iterate_to_convergence
|
||||
|
||||
TimeSlice<T> gains a pub(crate) convergence: ConvergenceOptions field
|
||||
set at construction. TimeSlice::new now takes it as a third parameter
|
||||
(breaking change to the pub constructor, acceptable in 0.1.x).
|
||||
History::add_events_with_prior passes self.convergence so the propagated
|
||||
value reaches every TimeSlice. The pre-existing convergence-the-method
|
||||
is renamed to iterate_to_convergence to disambiguate from the new
|
||||
convergence-the-field.
|
||||
|
||||
The field is wired but not yet read by inference — the three
|
||||
Game::*_with_arena callsites in time_slice.rs still hardcode
|
||||
ConvergenceOptions::default(). Task 2 changes that. Bit-equal because
|
||||
the propagated value equals the hardcoded value end-to-end.
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Read `self.convergence` at the three inference callsites
|
||||
|
||||
This task switches the three `Game::*_with_arena` callsites in `time_slice.rs` from hardcoded `ConvergenceOptions::default()` to the propagated `self.convergence` (or for `Event::iteration_direct`, a passed-in parameter). After this task, Damped EP set on `HistoryBuilder` actually reaches the within-game loop.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/time_slice.rs` (only)
|
||||
|
||||
- [ ] **Step 1: Add a `convergence` parameter to `Event::iteration_direct`**
|
||||
|
||||
In `src/time_slice.rs`, modify the existing `iteration_direct` signature (currently at `src/time_slice.rs:131-137`):
|
||||
|
||||
```rust
|
||||
fn iteration_direct<T: Time, D: Drift<T>>(
|
||||
&mut self,
|
||||
skills: &mut SkillStore,
|
||||
agents: &CompetitorStore<T, D>,
|
||||
p_draw: f64,
|
||||
convergence: crate::ConvergenceOptions,
|
||||
arena: &mut ScratchArena,
|
||||
) {
|
||||
```
|
||||
|
||||
Inside the body (around `src/time_slice.rs:140-156`), replace both `crate::ConvergenceOptions::default()` arguments with `convergence`:
|
||||
|
||||
```rust
|
||||
let g = match self.kind {
|
||||
EventKind::Ranked => Game::ranked_with_arena(
|
||||
teams,
|
||||
&result,
|
||||
&self.weights,
|
||||
p_draw,
|
||||
convergence,
|
||||
arena,
|
||||
),
|
||||
EventKind::Scored { score_sigma } => Game::scored_with_arena(
|
||||
teams,
|
||||
&result,
|
||||
&self.weights,
|
||||
score_sigma,
|
||||
convergence,
|
||||
arena,
|
||||
),
|
||||
};
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Update the rayon path in `sweep_color_groups` (cfg=rayon)**
|
||||
|
||||
In `src/time_slice.rs`, the rayon-feature `sweep_color_groups` (currently at `src/time_slice.rs:376-423`) captures `p_draw` by value into a `move` closure and calls `ev.iteration_direct(skills, agents, p_draw, &mut arena)`. Capture `convergence` the same way and pass it:
|
||||
|
||||
Above the rayon `for_each` at the line `let p_draw = self.p_draw;`, add:
|
||||
|
||||
```rust
|
||||
let convergence = self.convergence;
|
||||
```
|
||||
|
||||
Then update the call inside the closure (currently `ev.iteration_direct(skills, agents, p_draw, &mut arena);`):
|
||||
|
||||
```rust
|
||||
ev.iteration_direct(skills, agents, p_draw, convergence, &mut arena);
|
||||
```
|
||||
|
||||
The `else` branch (sequential fallback) at `src/time_slice.rs:417-421` calls `ev.iteration_direct(&mut self.skills, agents, p_draw, &mut self.arena);` — also update:
|
||||
|
||||
```rust
|
||||
ev.iteration_direct(&mut self.skills, agents, p_draw, self.convergence, &mut self.arena);
|
||||
```
|
||||
|
||||
(Note: this branch reads `self.convergence` directly because no `move` closure is involved here.)
|
||||
|
||||
- [ ] **Step 3: Update the non-rayon path in `sweep_color_groups`**
|
||||
|
||||
In `src/time_slice.rs`, the `#[cfg(not(feature = "rayon"))]` `sweep_color_groups` (currently at `src/time_slice.rs:428-444`) calls `ev.iteration_direct(&mut self.skills, agents, p_draw, &mut self.arena);` at `src/time_slice.rs:441`. Replace with:
|
||||
|
||||
```rust
|
||||
ev.iteration_direct(&mut self.skills, agents, p_draw, self.convergence, &mut self.arena);
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Update `TimeSlice::iteration`'s sequential branch**
|
||||
|
||||
In `src/time_slice.rs`, modify `TimeSlice::iteration` (at `src/time_slice.rs:322-363`). The sequential branch (when `from > 0 || self.color_groups.is_empty()`) has two `Game::*_with_arena` callsites at `src/time_slice.rs:330-346` that hardcode `crate::ConvergenceOptions::default()`. Replace both with `self.convergence`:
|
||||
|
||||
```rust
|
||||
let g = match event.kind {
|
||||
EventKind::Ranked => Game::ranked_with_arena(
|
||||
teams,
|
||||
&result,
|
||||
&event.weights,
|
||||
self.p_draw,
|
||||
self.convergence,
|
||||
&mut self.arena,
|
||||
),
|
||||
EventKind::Scored { score_sigma } => Game::scored_with_arena(
|
||||
teams,
|
||||
&result,
|
||||
&event.weights,
|
||||
score_sigma,
|
||||
self.convergence,
|
||||
&mut self.arena,
|
||||
),
|
||||
};
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Update `TimeSlice::log_evidence`**
|
||||
|
||||
In `src/time_slice.rs`, modify `TimeSlice::log_evidence` (at `src/time_slice.rs:505-540`). The two `Game::*_with_arena` callsites in the inner `run_event` closure at `src/time_slice.rs:519-538` hardcode `crate::ConvergenceOptions::default()`. Replace both with `self.convergence`:
|
||||
|
||||
```rust
|
||||
let run_event = |event: &Event, arena: &mut ScratchArena| -> f64 {
|
||||
let teams = event.within_priors(online, forward, &self.skills, agents);
|
||||
let result = event.outputs();
|
||||
match event.kind {
|
||||
EventKind::Ranked => Game::ranked_with_arena(
|
||||
teams,
|
||||
&result,
|
||||
&event.weights,
|
||||
self.p_draw,
|
||||
self.convergence,
|
||||
arena,
|
||||
)
|
||||
.evidence
|
||||
.ln(),
|
||||
EventKind::Scored { score_sigma } => Game::scored_with_arena(
|
||||
teams,
|
||||
&result,
|
||||
&event.weights,
|
||||
score_sigma,
|
||||
self.convergence,
|
||||
arena,
|
||||
)
|
||||
.evidence
|
||||
.ln(),
|
||||
}
|
||||
};
|
||||
```
|
||||
|
||||
(`self.convergence` is `Copy`, so the closure captures it by value naturally without needing a `let` binding outside.)
|
||||
|
||||
- [ ] **Step 6: Build and run the full test suite — bit-equal regression net**
|
||||
|
||||
Run: `cargo build && cargo test --lib`
|
||||
|
||||
Expected: all 98 lib tests still pass. Bit-equal goldens — every existing test uses `History::default()` or `HistoryBuilder::default()` (which sets `convergence = ConvergenceOptions::default()`), so the propagated value equals what the hardcoded default was. No test exercises a non-default convergence through History today, so no behavior changes.
|
||||
|
||||
If any test fails: investigate. The most likely cause is a stale `crate::ConvergenceOptions::default()` call missed in steps 1-5 — re-grep with `grep -n "ConvergenceOptions::default" src/time_slice.rs` to find any remaining hardcoded sites.
|
||||
|
||||
- [ ] **Step 7: Run integration tests**
|
||||
|
||||
Run: `cargo test`
|
||||
|
||||
Expected: all 27 integration tests still pass.
|
||||
|
||||
- [ ] **Step 8: Confirm no `crate::ConvergenceOptions::default()` remains in time_slice.rs**
|
||||
|
||||
Run: `grep -n "ConvergenceOptions::default" src/time_slice.rs`
|
||||
|
||||
Expected: only test-mod hits (in `TimeSlice::new(0i64, 0.0, ConvergenceOptions::default())` callsites from Task 1 step 5). NO production-code hits in `Event::iteration_direct`, `sweep_color_groups`, `TimeSlice::iteration`, or `TimeSlice::log_evidence`.
|
||||
|
||||
- [ ] **Step 9: Format and lint**
|
||||
|
||||
Run: `cargo +nightly fmt && cargo clippy --all-targets -- -D warnings`
|
||||
|
||||
Expected: no diff, no warnings.
|
||||
|
||||
- [ ] **Step 10: Commit**
|
||||
|
||||
```bash
|
||||
git add src/time_slice.rs
|
||||
git commit -m "$(cat <<'EOF'
|
||||
feat(time_slice): inference callsites read self.convergence
|
||||
|
||||
The three Game::*_with_arena callsites in time_slice.rs (in
|
||||
TimeSlice::iteration's sequential branch, TimeSlice::log_evidence's
|
||||
run_event closure, and Event::iteration_direct via parameter) now use
|
||||
the propagated ConvergenceOptions instead of hardcoded ::default().
|
||||
sweep_color_groups (both rayon and non-rayon paths) forwards
|
||||
self.convergence into Event::iteration_direct.
|
||||
|
||||
Damped EP (alpha < 1.0) and custom max_iter / epsilon set on
|
||||
HistoryBuilder::convergence(opts) now actually reach the within-game
|
||||
inference loop. Bit-equal for users on default options.
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Doc-comment update + end-to-end integration tests
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/convergence.rs` (alpha doc comment)
|
||||
- Modify: `src/history.rs` (two integration tests in the existing `#[cfg(test)] mod tests` block)
|
||||
|
||||
- [ ] **Step 1: Update `ConvergenceOptions::alpha` doc comment**
|
||||
|
||||
In `src/convergence.rs`, find the existing doc comment on the `alpha` field. Replace it with:
|
||||
|
||||
```rust
|
||||
/// EP damping factor in natural-parameter space: each per-factor
|
||||
/// update inside a single game writes `α·new + (1−α)·old`. `1.0` is
|
||||
/// undamped (default); `< 1.0` stabilises oscillating fixed-point
|
||||
/// loops at the cost of more iterations. Must be in `(0.0, 1.0]`.
|
||||
///
|
||||
/// Applies only to the within-game EP loop (`run_chain`). The outer
|
||||
/// `History::converge` cross-history sweep is undamped regardless of
|
||||
/// this value — cross-slice damping is a different concept and not
|
||||
/// in scope.
|
||||
pub alpha: f64,
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Locate the `#[cfg(test)] mod tests` block in `src/history.rs`**
|
||||
|
||||
Run: `grep -n "#\[cfg(test)\]" src/history.rs`
|
||||
|
||||
Identify the test module (there should be one near the bottom of the file). Read the imports at the top of that module so the new tests can reuse the existing test helpers and scope.
|
||||
|
||||
- [ ] **Step 3: Write the failing tests**
|
||||
|
||||
Add the following two tests at the end of the test module in `src/history.rs` (just before the module's closing `}`):
|
||||
|
||||
```rust
|
||||
#[test]
|
||||
fn history_propagates_convergence_to_inner_run_chain() {
|
||||
use crate::ConvergenceOptions;
|
||||
|
||||
// 4-team ranked game; each event needs more than one inner EP iter
|
||||
// to fully converge.
|
||||
let events_for = |h: &mut crate::History<i64, crate::drift::ConstantDrift,
|
||||
crate::observer::NullObserver, &'static str>| {
|
||||
for &name in &["a", "b", "c", "d"] {
|
||||
h.new_agent(name);
|
||||
}
|
||||
h.event(0)
|
||||
.team(["a"])
|
||||
.team(["b"])
|
||||
.team(["c"])
|
||||
.team(["d"])
|
||||
.commit()
|
||||
.unwrap();
|
||||
};
|
||||
|
||||
let mut h_capped = crate::History::builder()
|
||||
.convergence(ConvergenceOptions {
|
||||
max_iter: 1,
|
||||
..ConvergenceOptions::default()
|
||||
})
|
||||
.build();
|
||||
events_for(&mut h_capped);
|
||||
h_capped.converge().unwrap();
|
||||
|
||||
let mut h_full = crate::History::builder().build();
|
||||
events_for(&mut h_full);
|
||||
h_full.converge().unwrap();
|
||||
|
||||
let curves_capped = h_capped.learning_curves();
|
||||
let curves_full = h_full.learning_curves();
|
||||
|
||||
let mut max_diff: f64 = 0.0;
|
||||
for (key, capped_pts) in curves_capped.iter() {
|
||||
let full_pts = curves_full.get(key).expect("agent missing in full");
|
||||
for (capped, full) in capped_pts.iter().zip(full_pts.iter()) {
|
||||
max_diff = max_diff.max((capped.1.mu() - full.1.mu()).abs());
|
||||
max_diff = max_diff.max((capped.1.sigma() - full.1.sigma()).abs());
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
max_diff > 1e-6,
|
||||
"max_iter=1 inner loop should differ from default; max_diff={max_diff}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn history_with_damping_reaches_same_fixed_point_as_undamped() {
|
||||
use crate::ConvergenceOptions;
|
||||
|
||||
let events_for = |h: &mut crate::History<i64, crate::drift::ConstantDrift,
|
||||
crate::observer::NullObserver, &'static str>| {
|
||||
for &name in &["a", "b", "c", "d"] {
|
||||
h.new_agent(name);
|
||||
}
|
||||
h.event(0)
|
||||
.team(["a"])
|
||||
.team(["b"])
|
||||
.team(["c"])
|
||||
.team(["d"])
|
||||
.commit()
|
||||
.unwrap();
|
||||
};
|
||||
|
||||
let mut h_undamped = crate::History::builder().build();
|
||||
events_for(&mut h_undamped);
|
||||
h_undamped.converge().unwrap();
|
||||
|
||||
let mut h_damped = crate::History::builder()
|
||||
.convergence(ConvergenceOptions {
|
||||
alpha: 0.5,
|
||||
max_iter: 200,
|
||||
..ConvergenceOptions::default()
|
||||
})
|
||||
.build();
|
||||
events_for(&mut h_damped);
|
||||
h_damped.converge().unwrap();
|
||||
|
||||
let curves_u = h_undamped.learning_curves();
|
||||
let curves_d = h_damped.learning_curves();
|
||||
|
||||
let mut max_diff: f64 = 0.0;
|
||||
for (key, u_pts) in curves_u.iter() {
|
||||
let d_pts = curves_d.get(key).expect("agent missing in damped");
|
||||
for (u, d) in u_pts.iter().zip(d_pts.iter()) {
|
||||
max_diff = max_diff.max((u.1.mu() - d.1.mu()).abs());
|
||||
max_diff = max_diff.max((u.1.sigma() - d.1.sigma()).abs());
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
max_diff < 1e-3,
|
||||
"α=0.5 should reach the same fixed point as α=1.0; max_diff={max_diff}"
|
||||
);
|
||||
}
|
||||
```
|
||||
|
||||
If the import or method names (e.g. `History::builder()`, `event(...).team(...).commit()`, `learning_curves()`, `new_agent(...)`) don't match what's available in the test module, look at neighboring tests for the exact builder/event-construction pattern in current use and mirror it. The structure (build two Histories, add identical events, compare curves) is the contract; the surface syntax must follow what already works in this test file.
|
||||
|
||||
- [ ] **Step 4: Run the new tests**
|
||||
|
||||
Run: `cargo test --lib history_propagates_convergence_to_inner_run_chain history_with_damping_reaches_same_fixed_point_as_undamped`
|
||||
|
||||
Expected: 2 passed.
|
||||
|
||||
**Fallback if Test 1 fails** (`max_iter=1` produces the same posteriors as default — meaning the inner loop converges in one iteration on this graph): replace `max_iter: 1` with `max_iter: 0`. With `max_iter = 0` the inner loop body runs zero times, guaranteeing different posteriors than convergence.
|
||||
|
||||
**Fallback if Test 2 fails** (`max_diff` exceeds `1e-3`): raise `max_iter: 200` to `max_iter: 500`. Heavier damping needs more iterations to reach the same fixed point.
|
||||
|
||||
If neither fallback works, STOP and report BLOCKED with the actual `max_diff` and the iteration counts tried.
|
||||
|
||||
- [ ] **Step 5: Run the full test suite**
|
||||
|
||||
Run: `cargo test --lib && cargo test`
|
||||
|
||||
Expected: lib count = 100 (was 98), integration count = 27 (unchanged), all passing.
|
||||
|
||||
- [ ] **Step 6: Format and lint**
|
||||
|
||||
Run: `cargo +nightly fmt && cargo clippy --all-targets -- -D warnings`
|
||||
|
||||
Expected: no diff, no warnings.
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add src/convergence.rs src/history.rs
|
||||
git commit -m "$(cat <<'EOF'
|
||||
test(history): end-to-end ConvergenceOptions propagation tests
|
||||
|
||||
Two integration tests on a 4-team ranked event:
|
||||
- max_iter=1 set on HistoryBuilder produces measurably different
|
||||
posteriors than default, proving the inner loop honors the
|
||||
propagated max_iter
|
||||
- alpha=0.5 with extra iterations reaches the same fixed point as
|
||||
alpha=1.0, proving damping doesn't break correctness on the History
|
||||
path
|
||||
|
||||
Also updates the alpha doc comment to clarify it applies only to the
|
||||
within-game EP loop, not the outer cross-history sweep.
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Self-review (writer's note)
|
||||
|
||||
**Spec coverage:**
|
||||
- Spec § "What ships" item 1 (TimeSlice convergence field) → Task 1 step 2 ✓
|
||||
- Spec § "What ships" item 2 (TimeSlice::new signature) → Task 1 step 3 ✓
|
||||
- Spec § "What ships" item 3 (History passes self.convergence) → Task 1 step 4 ✓
|
||||
- Spec § "What ships" item 4 (Event::iteration_direct gains parameter) → Task 2 step 1 ✓
|
||||
- Spec § "What ships" item 4 (callers pass self.convergence) → Task 2 steps 2, 3 ✓
|
||||
- Spec § "What ships" item 5 (TimeSlice::convergence-method reads field) → Task 2 step 4 ✓
|
||||
- Spec § "What ships" item 6 (log_evidence reads field) → Task 2 step 5 ✓
|
||||
- Spec § "What ships" item 7 (test callsite updates) → Task 1 step 5 ✓
|
||||
- Spec § "Design" rename method → Task 1 step 6 ✓
|
||||
- Spec § "Risks" alpha doc-comment update → Task 3 step 1 ✓
|
||||
- Spec § "Testing strategy" §1 (regression net) → Tasks 1 step 7, 2 step 6, 3 step 5 ✓
|
||||
- Spec § "Testing strategy" §2 (history_propagates_convergence) → Task 3 step 3 test 1 ✓
|
||||
- Spec § "Testing strategy" §2 (history_with_damping_reaches_same_fixed_point) → Task 3 step 3 test 2 ✓
|
||||
|
||||
**Out-of-scope items correctly absent:** No new `History`/`HistoryBuilder` methods, no `ConvergenceOptions` split, no `Damped` Schedule impl, no nat-param convergence switch.
|
||||
|
||||
**Type / signature consistency:**
|
||||
- `TimeSlice::new(time, p_draw, convergence: ConvergenceOptions)` — Task 1 step 3 (def) and Task 1 step 4-5 (call sites) match ✓
|
||||
- `iteration_direct(skills, agents, p_draw, convergence, arena)` — Task 2 step 1 (def) and steps 2, 3 (call sites) match ✓
|
||||
- `iterate_to_convergence` — Task 1 step 6 ✓
|
||||
- All `self.convergence` reads are field accesses, not method calls (the rename in Task 1 step 6 prevents ambiguity) ✓
|
||||
|
||||
**Two tasks (1 and 2) split rationale:** Task 1 wires the field but the inference path still uses hardcoded defaults (no behavioral change). Task 2 makes the field actually drive inference (behavioral change for non-default users). Each task is independently committable and the test suite is bit-equal at every checkpoint.
|
||||
|
||||
**No placeholders detected.**
|
||||
@@ -0,0 +1,444 @@
|
||||
# Tech Debt Cleanup Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Land three independent post-T4-MarginFactor cleanups: dedupe `Game::likelihoods` and `Game::likelihoods_scored` via a `run_chain` helper, make `BuiltinFactor::log_evidence` exhaustive, and fix stale numerics in the T4 plan doc.
|
||||
|
||||
**Architecture:** Pure code-shape and doc fixes. No public-API change, no behavioral change, no new dependencies. The dedup is a pure refactor — bit-equal posteriors and evidence against existing test goldens. The exhaustive match is a future-proofing change with no runtime effect. The doc fix is two number swaps in prose plus one matching code-comment swap.
|
||||
|
||||
**Tech Stack:** Rust 2024, `cargo +nightly fmt`, `cargo clippy`, `cargo test --lib`.
|
||||
|
||||
---
|
||||
|
||||
## Spec reference
|
||||
|
||||
`docs/superpowers/specs/2026-05-08-tech-debt-cleanup-design.md`
|
||||
|
||||
## File map
|
||||
|
||||
| File | Why touched |
|
||||
|---|---|
|
||||
| `src/game.rs` | Add `run_chain` helper; rewrite `likelihoods` and `likelihoods_scored` to call it |
|
||||
| `src/factor/mod.rs` | Make `BuiltinFactor::log_evidence` match exhaustive |
|
||||
| `docs/superpowers/plans/2026-04-27-t4-margin-factor.md` | Fix two stale prose numbers and one matching code comment |
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Extract `run_chain` helper, dedupe both likelihoods methods
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/game.rs:236-485` (replace both `likelihoods` and `likelihoods_scored` with one helper + two thin callers)
|
||||
|
||||
**Context for the implementer (read this before touching anything):**
|
||||
|
||||
`OwnedGame<T, D>` (defined at `src/game.rs:83-92`) holds `teams`, `result`, `weights`, `p_draw`, plus mutable output fields `likelihoods: Vec<Vec<Gaussian>>` and `evidence: f64`. Two private methods on `Game<'a, T, D>` (the borrowed sibling at `src/game.rs:148-156`) compute likelihoods:
|
||||
|
||||
- `likelihoods(&mut self, arena: &mut ScratchArena)` — ranked outcomes; `src/game.rs:236-371`
|
||||
- `likelihoods_scored(&mut self, arena: &mut ScratchArena, score_sigma: f64)` — scored outcomes; `src/game.rs:373-485`
|
||||
|
||||
The two are bit-identical except for the closure that builds the per-diff `DiffFactor` (defined at `src/game.rs:20-54`). `DiffFactor` has two variants: `Trunc(TruncFactor)` for ranked, `Margin(MarginFactor)` for scored.
|
||||
|
||||
The shared body does, in order: `arena.reset()`, sort teams descending by `result` into `arena.sort_buf`, fill `arena.team_prior`, build `links: Vec<DiffFactor>` (the differing block), resize `arena.lhood_lose` / `arena.lhood_win` to `N_INF`, run a forward+backward sweep with a max-iter-10 fixed-point loop guarded by `tuple_gt(step, 1e-6)`, handle the `n_diffs == 1` special case, do boundary updates, multiply per-diff `evidence()` into `self.evidence`, build the inverse permutation in `arena.inv_buf`, then build `self.likelihoods` from the per-team `lhood_win * lhood_lose` and per-player `performance().exclude(...).forget(beta²)` math.
|
||||
|
||||
**Refactor target:**
|
||||
|
||||
```rust
|
||||
fn run_chain<F>(
|
||||
&self,
|
||||
arena: &mut ScratchArena,
|
||||
mut make_link: F,
|
||||
) -> (f64, Vec<Vec<Gaussian>>)
|
||||
where
|
||||
F: FnMut(usize, &[usize], &mut crate::factor::VarStore) -> DiffFactor,
|
||||
{ /* the entire shared body, returning (evidence, likelihoods) */ }
|
||||
```
|
||||
|
||||
Helper takes `&self` (not `&mut self`) so the closure can capture `&self.result`, `&self.teams`, `&self.weights`, `&self.p_draw` without conflicting with the helper's own immutable borrow. The arena is borrowed `&mut` independently.
|
||||
|
||||
The closure is invoked once per diff index `i ∈ 0..n_diffs`, after `arena.sort_buf` is filled. It receives `i`, `&arena.sort_buf[..]`, and `&mut arena.vars` so it can `alloc(N_INF)` the diff `VarId`. It returns the constructed `DiffFactor`.
|
||||
|
||||
The two callers shrink to:
|
||||
|
||||
```rust
|
||||
fn likelihoods(&mut self, arena: &mut ScratchArena) {
|
||||
let p_draw = self.p_draw;
|
||||
let result = &self.result;
|
||||
let teams = &self.teams;
|
||||
let (evidence, likelihoods) = Self::dummy_to_satisfy_borrowck(/* see below */);
|
||||
// ... assigns self.evidence and self.likelihoods
|
||||
}
|
||||
```
|
||||
|
||||
Wait — actually borrow-checker note: calling `self.run_chain(arena, |i, sort_buf, vars| { use_self_fields })` from a `&mut self` method is **fine** because `run_chain` takes `&self` and the closure captures `&self` immutably. Both share an immutable reborrow of `*self`. The arena is a separate `&mut` borrow. Verify the implementer doesn't accidentally make `run_chain` take `&mut self`.
|
||||
|
||||
**Why a closure (not a trait, not a two-phase build).** A closure keeps caller-specific state (`p_draw`, `score_sigma`, beta sums) inline at the call site with zero ceremony. A trait would require a stateful builder per call. A two-phase build (caller produces `Vec<DiffFactor>` first, helper does the rest) would either re-do the sort or split arena ownership awkwardly between the phases.
|
||||
|
||||
---
|
||||
|
||||
- [ ] **Step 1: Run the existing test suite to capture the baseline**
|
||||
|
||||
Run: `cargo test --lib`
|
||||
|
||||
Expected: all tests pass. Note the count (should be 88+ lib tests) — the refactor must keep this number unchanged with all green.
|
||||
|
||||
- [ ] **Step 2: Open `src/game.rs` and add the `run_chain` helper**
|
||||
|
||||
Inside `impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> { ... }` (the block starting at `src/game.rs:158`), add `run_chain` immediately above the existing `likelihoods` method (so above line 236). Use exactly this body — it is the merge of the two existing methods with the differing block replaced by the closure call:
|
||||
|
||||
```rust
|
||||
fn run_chain<F>(
|
||||
&self,
|
||||
arena: &mut ScratchArena,
|
||||
mut make_link: F,
|
||||
) -> (f64, Vec<Vec<Gaussian>>)
|
||||
where
|
||||
F: FnMut(usize, &[usize], &mut crate::factor::VarStore) -> DiffFactor,
|
||||
{
|
||||
arena.reset();
|
||||
|
||||
let n_teams = self.teams.len();
|
||||
|
||||
arena.sort_buf.extend(0..n_teams);
|
||||
arena.sort_buf.sort_by(|&i, &j| {
|
||||
self.result[j]
|
||||
.partial_cmp(&self.result[i])
|
||||
.unwrap_or(Ordering::Equal)
|
||||
});
|
||||
|
||||
arena.team_prior.extend(arena.sort_buf.iter().map(|&t| {
|
||||
self.teams[t]
|
||||
.iter()
|
||||
.zip(self.weights[t].iter())
|
||||
.fold(N00, |p, (player, &w)| p + (player.performance() * w))
|
||||
}));
|
||||
|
||||
let n_diffs = n_teams.saturating_sub(1);
|
||||
|
||||
let mut links: Vec<DiffFactor> = (0..n_diffs)
|
||||
.map(|i| make_link(i, &arena.sort_buf, &mut arena.vars))
|
||||
.collect();
|
||||
|
||||
arena.lhood_lose.resize(n_teams, N_INF);
|
||||
arena.lhood_win.resize(n_teams, N_INF);
|
||||
|
||||
let mut step = (f64::INFINITY, f64::INFINITY);
|
||||
let mut iter = 0;
|
||||
|
||||
while tuple_gt(step, 1e-6) && iter < 10 {
|
||||
step = (0.0_f64, 0.0_f64);
|
||||
|
||||
for (e, lf) in links[..n_diffs.saturating_sub(1)].iter_mut().enumerate() {
|
||||
let pw = arena.team_prior[e] * arena.lhood_lose[e];
|
||||
let pl = arena.team_prior[e + 1] * arena.lhood_win[e + 1];
|
||||
let raw = pw - pl;
|
||||
arena.vars.set(lf.diff(), raw * lf.msg());
|
||||
let d = lf.propagate(&mut arena.vars);
|
||||
step = tuple_max(step, d);
|
||||
|
||||
let new_ll = pw - lf.msg();
|
||||
step = tuple_max(step, arena.lhood_lose[e + 1].delta(new_ll));
|
||||
arena.lhood_lose[e + 1] = new_ll;
|
||||
}
|
||||
|
||||
for (rev_i, lf) in links[1..].iter_mut().rev().enumerate() {
|
||||
let e = n_diffs - 1 - rev_i;
|
||||
let pw = arena.team_prior[e] * arena.lhood_lose[e];
|
||||
let pl = arena.team_prior[e + 1] * arena.lhood_win[e + 1];
|
||||
let raw = pw - pl;
|
||||
arena.vars.set(lf.diff(), raw * lf.msg());
|
||||
let d = lf.propagate(&mut arena.vars);
|
||||
step = tuple_max(step, d);
|
||||
|
||||
let new_lw = pl + lf.msg();
|
||||
step = tuple_max(step, arena.lhood_win[e].delta(new_lw));
|
||||
arena.lhood_win[e] = new_lw;
|
||||
}
|
||||
|
||||
iter += 1;
|
||||
}
|
||||
|
||||
if n_diffs == 1 {
|
||||
let raw = (arena.team_prior[0] * arena.lhood_lose[0])
|
||||
- (arena.team_prior[1] * arena.lhood_win[1]);
|
||||
arena.vars.set(links[0].diff(), raw * links[0].msg());
|
||||
links[0].propagate(&mut arena.vars);
|
||||
}
|
||||
|
||||
if n_diffs > 0 {
|
||||
let pl1 = arena.team_prior[1] * arena.lhood_win[1];
|
||||
arena.lhood_win[0] = pl1 + links[0].msg();
|
||||
let pw_last = arena.team_prior[n_teams - 2] * arena.lhood_lose[n_teams - 2];
|
||||
arena.lhood_lose[n_teams - 1] = pw_last - links[n_diffs - 1].msg();
|
||||
}
|
||||
|
||||
let evidence: f64 = links.iter().map(|l| l.evidence()).product();
|
||||
|
||||
arena.inv_buf.resize(n_teams, 0);
|
||||
for (si, &orig_i) in arena.sort_buf.iter().enumerate() {
|
||||
arena.inv_buf[orig_i] = si;
|
||||
}
|
||||
|
||||
let likelihoods = self
|
||||
.teams
|
||||
.iter()
|
||||
.zip(self.weights.iter())
|
||||
.enumerate()
|
||||
.map(|(orig_i, (players, weights))| {
|
||||
let si = arena.inv_buf[orig_i];
|
||||
let m = arena.lhood_win[si] * arena.lhood_lose[si];
|
||||
let performance = players
|
||||
.iter()
|
||||
.zip(weights.iter())
|
||||
.fold(N00, |p, (player, &w)| p + (player.performance() * w));
|
||||
players
|
||||
.iter()
|
||||
.zip(weights.iter())
|
||||
.map(|(player, &w)| {
|
||||
((m - performance.exclude(player.performance() * w)) * (1.0 / w))
|
||||
.forget(player.beta.powi(2))
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
(evidence, likelihoods)
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Replace `likelihoods` body with a thin caller**
|
||||
|
||||
In `src/game.rs`, replace the entire body of `fn likelihoods(&mut self, arena: &mut ScratchArena)` (currently lines 236-371 — replace from the opening `{` to the closing `}` of that method) with:
|
||||
|
||||
```rust
|
||||
fn likelihoods(&mut self, arena: &mut ScratchArena) {
|
||||
let p_draw = self.p_draw;
|
||||
// Capture pointers to fields the closure reads, to keep borrow scopes tight.
|
||||
// Closure captures &self.result and &self.teams (both immutable) and the
|
||||
// &mut arena passed in via run_chain — disjoint from `&self`.
|
||||
let (evidence, likelihoods) = self.run_chain(arena, |i, sort_buf, vars| {
|
||||
let tie = self.result[sort_buf[i]] == self.result[sort_buf[i + 1]];
|
||||
let margin = if p_draw == 0.0 {
|
||||
0.0
|
||||
} else {
|
||||
let a: f64 = self.teams[sort_buf[i]]
|
||||
.iter()
|
||||
.map(|p| p.beta.powi(2))
|
||||
.sum();
|
||||
let b: f64 = self.teams[sort_buf[i + 1]]
|
||||
.iter()
|
||||
.map(|p| p.beta.powi(2))
|
||||
.sum();
|
||||
compute_margin(p_draw, (a + b).sqrt())
|
||||
};
|
||||
let vid = vars.alloc(N_INF);
|
||||
DiffFactor::Trunc(TruncFactor::new(vid, margin, tie))
|
||||
});
|
||||
self.evidence = evidence;
|
||||
self.likelihoods = likelihoods;
|
||||
}
|
||||
```
|
||||
|
||||
(Capturing `p_draw` as a local binding before the closure avoids a `self.p_draw` borrow inside; it's a `Copy` `f64` so this is free.)
|
||||
|
||||
- [ ] **Step 4: Replace `likelihoods_scored` body with a thin caller**
|
||||
|
||||
In `src/game.rs`, replace the entire body of `fn likelihoods_scored(&mut self, arena: &mut ScratchArena, score_sigma: f64)` (currently lines 373-485) with:
|
||||
|
||||
```rust
|
||||
fn likelihoods_scored(&mut self, arena: &mut ScratchArena, score_sigma: f64) {
|
||||
let (evidence, likelihoods) = self.run_chain(arena, |i, sort_buf, vars| {
|
||||
// After descending-by-score sort, m_obs >= 0 for every adjacent pair.
|
||||
let m_obs = self.result[sort_buf[i]] - self.result[sort_buf[i + 1]];
|
||||
let vid = vars.alloc(N_INF);
|
||||
DiffFactor::Margin(MarginFactor::new(vid, m_obs, score_sigma))
|
||||
});
|
||||
self.evidence = evidence;
|
||||
self.likelihoods = likelihoods;
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Build to confirm it compiles**
|
||||
|
||||
Run: `cargo build`
|
||||
|
||||
Expected: compiles cleanly. If the borrow checker complains that the closure conflicts with `self.run_chain(...)`, the most likely cause is `run_chain` accidentally being `&mut self` — confirm its signature is `fn run_chain<F>(&self, arena: &mut ScratchArena, mut make_link: F) -> (f64, Vec<Vec<Gaussian>>)`. If that's correct and there's still a conflict, double-check the closure's captures: it should capture `&self.result` and `&self.teams` (immutable), `p_draw: f64` by value (Copy), and `score_sigma: f64` by value (Copy). It must NOT touch `&mut self` in any form.
|
||||
|
||||
- [ ] **Step 6: Run the full library test suite — must be all green, same count as Step 1**
|
||||
|
||||
Run: `cargo test --lib`
|
||||
|
||||
Expected: same number of tests as Step 1, all pass. Bit-equal goldens — every existing assertion (`test_1vs1`, `test_1vs1_draw`, `test_2vs1vs2_mixed`, MarginFactor end-to-end tests, etc.) must pass unchanged. If ANY test fails, the refactor is wrong; revert and re-inspect.
|
||||
|
||||
- [ ] **Step 7: Run integration tests too**
|
||||
|
||||
Run: `cargo test`
|
||||
|
||||
Expected: all integration tests pass (28 noted in commit `8b53cac`).
|
||||
|
||||
- [ ] **Step 8: Format and lint**
|
||||
|
||||
Run: `cargo +nightly fmt && cargo clippy --lib -- -D warnings`
|
||||
|
||||
Expected: no diffs from fmt, no clippy warnings.
|
||||
|
||||
- [ ] **Step 9: Commit**
|
||||
|
||||
```bash
|
||||
git add src/game.rs
|
||||
git commit -m "$(cat <<'EOF'
|
||||
refactor: dedupe Game::likelihoods and likelihoods_scored via run_chain
|
||||
|
||||
Both methods were 95-line near-duplicates differing only in the closure
|
||||
that builds the per-diff DiffFactor. Extract the shared body as a
|
||||
private run_chain<F>(&self, arena, make_link) helper that returns
|
||||
(evidence, likelihoods); the two callers shrink to ~10 lines each.
|
||||
|
||||
Pure code-shape change: posteriors and evidence remain bit-equal; all
|
||||
existing tests (lib + integration) pass unchanged.
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Make `BuiltinFactor::log_evidence` match exhaustive
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/factor/mod.rs:94-100` (the `log_evidence` impl on `BuiltinFactor`)
|
||||
|
||||
- [ ] **Step 1: Open `src/factor/mod.rs` and replace the `log_evidence` body**
|
||||
|
||||
Replace the existing impl:
|
||||
|
||||
```rust
|
||||
fn log_evidence(&self, vars: &VarStore) -> f64 {
|
||||
match self {
|
||||
Self::Trunc(f) => f.log_evidence(vars),
|
||||
Self::Margin(f) => f.log_evidence(vars),
|
||||
_ => 0.0,
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```rust
|
||||
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,
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Build and run tests**
|
||||
|
||||
Run: `cargo build && cargo test --lib`
|
||||
|
||||
Expected: compiles cleanly, all tests pass. Behavior is unchanged — `TeamSum` and `RankDiff` still return `0.0`, but a future variant will now produce a non-exhaustive-match error instead of being silently swallowed.
|
||||
|
||||
- [ ] **Step 3: Format and lint**
|
||||
|
||||
Run: `cargo +nightly fmt && cargo clippy --lib -- -D warnings`
|
||||
|
||||
Expected: no diffs, no warnings.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add src/factor/mod.rs
|
||||
git commit -m "$(cat <<'EOF'
|
||||
refactor: make BuiltinFactor::log_evidence match exhaustive
|
||||
|
||||
Replace the `_ => 0.0` wildcard with explicit
|
||||
`Self::TeamSum(_) | Self::RankDiff(_) => 0.0`. No behavioral change;
|
||||
future variants now produce a compile error instead of being silently
|
||||
absorbed by the wildcard.
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Fix stale numerics in T4 plan doc
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/superpowers/plans/2026-04-27-t4-margin-factor.md` (lines 52 and 185)
|
||||
|
||||
The shipped test in `src/factor/mod.rs:163,166` asserts:
|
||||
|
||||
```
|
||||
assert!((result.mu() - 4.864864864864865).abs() < 1e-12);
|
||||
assert!((logz - (-3.062235327364623)).abs() < 1e-10);
|
||||
```
|
||||
|
||||
The plan's prose at line 52 quotes pre-shipped values that no longer match. This task fixes the prose and the matching code-comment. The full-precision assertion blocks elsewhere in the plan are out of scope (they belong to the plan-as-written, and the spec's fix table only listed the rounded prose values).
|
||||
|
||||
- [ ] **Step 1: Update the prose at line 52**
|
||||
|
||||
Open `docs/superpowers/plans/2026-04-27-t4-margin-factor.md`. Find the line:
|
||||
|
||||
```
|
||||
- `Z_cav = pdf(5, 0, sqrt(36 + 1)) = pdf(5, 0, sqrt(37)) ≈ 0.046827`. So `log_evidence ≈ -3.0613`.
|
||||
```
|
||||
|
||||
Replace with:
|
||||
|
||||
```
|
||||
- `Z_cav = pdf(5, 0, sqrt(36 + 1)) = pdf(5, 0, sqrt(37)) ≈ 0.04678`. So `log_evidence ≈ -3.0622`.
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Update the matching code-comment at line 185**
|
||||
|
||||
In the same file, find:
|
||||
|
||||
```
|
||||
// pdf(5, 0, sqrt(37)) ≈ 0.046827
|
||||
```
|
||||
|
||||
Replace with:
|
||||
|
||||
```
|
||||
// pdf(5, 0, sqrt(37)) ≈ 0.04678
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Verify nothing else changed**
|
||||
|
||||
Run: `git diff docs/superpowers/plans/2026-04-27-t4-margin-factor.md`
|
||||
|
||||
Expected: exactly three lines changed (one prose line containing both numbers, one comment line). Nothing else should be touched.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add docs/superpowers/plans/2026-04-27-t4-margin-factor.md
|
||||
git commit -m "$(cat <<'EOF'
|
||||
docs: fix stale numerics in t4-margin-factor plan
|
||||
|
||||
The plan's prose quoted Z_cav ≈ 0.046827 and log_evidence ≈ -3.0613,
|
||||
which diverged from the values asserted by the shipped test in
|
||||
src/factor/mod.rs (-3.062235327364623). Update prose and the matching
|
||||
code comment to 0.04678 / -3.0622.
|
||||
EOF
|
||||
)"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Self-review (writer's note)
|
||||
|
||||
Spec coverage:
|
||||
- Spec Item 1 (dedupe `likelihoods`/`likelihoods_scored`) → Task 1 ✓
|
||||
- Spec Item 2 (exhaustive `BuiltinFactor::log_evidence`) → Task 2 ✓
|
||||
- Spec Item 3 (stale numerics in T4 plan) → Task 3 ✓
|
||||
- Spec out-of-scope items (`DiffFactor` collapse, per-event `score_sigma`) — correctly absent ✓
|
||||
|
||||
Verification gates per the spec ("each item commits independently and ships behind a green `cargo test --lib`"): every task ends in fmt + clippy + tests + commit. Task 1 additionally runs `cargo test` for integration coverage.
|
||||
|
||||
Type / signature consistency:
|
||||
- `run_chain` signature appears identically in the context header and Step 2 body ✓
|
||||
- Closure type `FnMut(usize, &[usize], &mut crate::factor::VarStore) -> DiffFactor` matches across Step 2 (definition) and Steps 3/4 (call sites) ✓
|
||||
- `DiffFactor::Trunc` / `DiffFactor::Margin` constructors match `src/game.rs:20-23` definitions ✓
|
||||
|
||||
No placeholders detected.
|
||||
@@ -0,0 +1,320 @@
|
||||
# Damped EP — Game-Local Damping
|
||||
|
||||
## Summary
|
||||
|
||||
Add an opt-in EP damping knob to within-game inference. Users set
|
||||
`ConvergenceOptions::alpha < 1.0` to damp message updates and stabilise
|
||||
oscillating fixed-point loops on hard graphs. `alpha = 1.0` (the default)
|
||||
is bit-equal to today.
|
||||
|
||||
This is the smallest-scope realisation of the spec's `Damped` schedule:
|
||||
**game-local**, not plumbed through the `Schedule` trait. The `Schedule`
|
||||
trait is shipped infrastructure that `run_chain` does not currently call;
|
||||
wiring `Schedule` into game inference is a separate future task. This
|
||||
design touches only what the user can actually reach via `GameOptions`.
|
||||
|
||||
## Scope
|
||||
|
||||
### What ships
|
||||
|
||||
1. New field `ConvergenceOptions::alpha: f64` (default `1.0`).
|
||||
2. `run_chain` reads `options.convergence.{epsilon, max_iter, alpha}`
|
||||
instead of the hardcoded `1e-6` / `10` / undamped — fixes the existing
|
||||
latent bug where the first two were already on `GameOptions` but never
|
||||
read by inference.
|
||||
3. `Gaussian::damp_natural(self, new, alpha) -> Gaussian` — public helper
|
||||
computing `α·new + (1−α)·self` in natural-parameter space.
|
||||
4. `TruncFactor` and `MarginFactor` gain inherent
|
||||
`propagate_with_alpha(&mut self, vars, alpha) -> (f64, f64)`. Their
|
||||
`Factor::propagate` impls become one-line delegations passing
|
||||
`alpha = 1.0`.
|
||||
5. `DiffFactor::propagate` (game-private enum at `src/game.rs:20-54`)
|
||||
gains an `alpha: f64` parameter and dispatches into the underlying
|
||||
factor's `propagate_with_alpha`.
|
||||
|
||||
### What does not ship
|
||||
|
||||
- No `Damped` impl in `src/schedule.rs`. The `Schedule` trait stays as
|
||||
it is; integration with `run_chain` is a separate task.
|
||||
- No nat-param convergence switch. `(|Δmu|, |Δsigma|)` stays the
|
||||
delta basis (matches today). The spec's "stopping in natural-param
|
||||
space" wants its own design pass and test re-tuning.
|
||||
- No oscillation auto-detect. `alpha` is user-supplied and constant for
|
||||
the duration of a `run_chain` call.
|
||||
- No `Residual`, `OneShot`, or `SynergyFactor` / `ScoreFactor` work —
|
||||
separate future plans.
|
||||
|
||||
## Design
|
||||
|
||||
### `ConvergenceOptions::alpha`
|
||||
|
||||
```rust
|
||||
// src/convergence.rs
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
pub struct ConvergenceOptions {
|
||||
pub max_iter: usize,
|
||||
pub epsilon: f64,
|
||||
pub alpha: f64,
|
||||
}
|
||||
|
||||
impl Default for ConvergenceOptions {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
max_iter: crate::ITERATIONS,
|
||||
epsilon: crate::EPSILON,
|
||||
alpha: 1.0,
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`alpha = 1.0` ⇒ undamped (bit-equal to today). Recommended starting
|
||||
point if a graph oscillates: `0.5`–`0.7`. Values approaching `0.0` make
|
||||
each step tinier and slow convergence; `alpha = 0.0` is degenerate
|
||||
(factor never updates). Validation in `run_chain`:
|
||||
|
||||
```rust
|
||||
debug_assert!(
|
||||
opts.convergence.alpha > 0.0 && opts.convergence.alpha <= 1.0,
|
||||
"convergence alpha must be in (0.0, 1.0]"
|
||||
);
|
||||
```
|
||||
|
||||
### `Gaussian::damp_natural`
|
||||
|
||||
```rust
|
||||
impl Gaussian {
|
||||
/// EP damping in natural-parameter space: `α·new + (1−α)·self`.
|
||||
///
|
||||
/// Used by within-game schedules to stabilise oscillating fixed-point
|
||||
/// loops on hard graphs. `alpha = 1.0` returns `new` exactly;
|
||||
/// `alpha < 1.0` shrinks each per-step update.
|
||||
pub fn damp_natural(self, new: Gaussian, alpha: f64) -> Gaussian {
|
||||
Gaussian::from_natural(
|
||||
alpha * new.pi() + (1.0 - alpha) * self.pi(),
|
||||
alpha * new.tau() + (1.0 - alpha) * self.tau(),
|
||||
)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Public on `Gaussian`. The name encodes the WHY (EP damping); the doc
|
||||
comment fixes the math. No new dependency.
|
||||
|
||||
The existing `Mul<f64> for Gaussian` is **distribution scaling**
|
||||
(`sigma → sigma·|scalar|`), not nat-param interpolation, so it can't be
|
||||
reused here.
|
||||
|
||||
### `TruncFactor::propagate_with_alpha`
|
||||
|
||||
```rust
|
||||
impl TruncFactor {
|
||||
pub(crate) fn propagate_with_alpha(
|
||||
&mut self,
|
||||
vars: &mut VarStore,
|
||||
alpha: f64,
|
||||
) -> (f64, f64) {
|
||||
let marginal = vars.get(self.diff);
|
||||
let cavity = marginal / self.msg;
|
||||
|
||||
if self.evidence_cached.is_none() {
|
||||
self.evidence_cached = Some(cavity_evidence(cavity, self.margin, self.tie));
|
||||
}
|
||||
|
||||
let trunc = approx(cavity, self.margin, self.tie);
|
||||
let new_msg = trunc / cavity;
|
||||
|
||||
let damped = self.msg.damp_natural(new_msg, alpha);
|
||||
let old_msg = self.msg;
|
||||
self.msg = damped;
|
||||
|
||||
// marginal_new = cavity * stored_msg (NOT cavity * new_msg with damping)
|
||||
vars.set(self.diff, cavity * damped);
|
||||
|
||||
old_msg.delta(damped)
|
||||
}
|
||||
}
|
||||
|
||||
impl Factor for TruncFactor {
|
||||
fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64) {
|
||||
self.propagate_with_alpha(vars, 1.0)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Two important points:
|
||||
|
||||
- The variable receives `cavity * damped` (i.e. `cavity * self.msg`),
|
||||
not `trunc`. With `alpha = 1.0` these are equal (since
|
||||
`cavity * new_msg = trunc` by construction), so today's behaviour is
|
||||
preserved bit-equal. With `alpha < 1.0` the marginal reflects the
|
||||
partially-applied update.
|
||||
- The reported delta is `old_msg.delta(damped)` — delta of the actually
|
||||
stored message, not of the raw `new_msg`. This is the textbook EP
|
||||
damping convention: the convergence loop measures the trajectory it
|
||||
is actually walking.
|
||||
|
||||
`MarginFactor` follows the same shape, with its own
|
||||
`propagate_with_alpha` body (the existing `propagate` math, with the
|
||||
`damp_natural` step inserted in the same place and the var write
|
||||
switched to `cavity * damped`).
|
||||
|
||||
### `DiffFactor::propagate` signature
|
||||
|
||||
```rust
|
||||
// src/game.rs
|
||||
impl DiffFactor {
|
||||
pub(crate) fn propagate(
|
||||
&mut self,
|
||||
vars: &mut VarStore,
|
||||
alpha: f64,
|
||||
) -> (f64, f64) {
|
||||
match self {
|
||||
Self::Trunc(f) => f.propagate_with_alpha(vars, alpha),
|
||||
Self::Margin(f) => f.propagate_with_alpha(vars, alpha),
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`DiffFactor` is `pub(crate)` and only used inside `run_chain`, so the
|
||||
signature change has no public-API impact.
|
||||
|
||||
### `run_chain` changes
|
||||
|
||||
Inside `Game::run_chain` (`src/game.rs:236-348`):
|
||||
|
||||
1. Capture `let alpha = opts.convergence.alpha;` once at the top
|
||||
(avoids repeated `opts.convergence.alpha` lookups in the hot loop).
|
||||
2. Replace the loop guard
|
||||
`while tuple_gt(step, 1e-6) && iter < 10`
|
||||
with
|
||||
`while tuple_gt(step, opts.convergence.epsilon) && iter < opts.convergence.max_iter`.
|
||||
3. Replace each `lf.propagate(&mut arena.vars)` call site (three of
|
||||
them: forward sweep, backward sweep, `n_diffs == 1` special case)
|
||||
with `lf.propagate(&mut arena.vars, alpha)`.
|
||||
|
||||
The threading of `opts: &GameOptions` into `run_chain` is the only
|
||||
new caller obligation. Today `run_chain` doesn't take `opts`; the two
|
||||
callers (`likelihoods`, `likelihoods_scored`) currently invoke it
|
||||
without options. Both will need to pass the options through. The
|
||||
`Game<'a, T, D>` struct does not currently hold `GameOptions`; the
|
||||
options are constructed and discarded around the call to
|
||||
`{ranked,scored}_with_arena`. So:
|
||||
|
||||
- `Game::ranked_with_arena` and `Game::scored_with_arena` already
|
||||
receive `p_draw` / `score_sigma` as scalar params; we extend them to
|
||||
accept `&ConvergenceOptions` (or the full `&GameOptions`) too.
|
||||
- `likelihoods` / `likelihoods_scored` either store the options on
|
||||
`Game` or accept them as method parameters and forward to
|
||||
`run_chain`.
|
||||
|
||||
The simplest plumbing: store `convergence: ConvergenceOptions` as a
|
||||
field on `Game<'a, T, D>` and `OwnedGame<T, D>` populated at
|
||||
construction time. Then `run_chain` can read it from `&self`.
|
||||
|
||||
## Convergence semantics
|
||||
|
||||
With `alpha < 1.0` the per-step update shrinks; convergence may take
|
||||
more iterations to reach the same `epsilon` threshold. Users who damp
|
||||
should also raise `max_iter` accordingly. Documentation example:
|
||||
|
||||
```rust
|
||||
let mut opts = GameOptions::default();
|
||||
opts.convergence.alpha = 0.5;
|
||||
opts.convergence.max_iter = 30;
|
||||
```
|
||||
|
||||
## Testing strategy
|
||||
|
||||
### Regression net (no new file)
|
||||
|
||||
The existing 88 lib tests and 27 integration tests are the bit-equal
|
||||
regression net. With `alpha = 1.0` (the default), every assertion must
|
||||
pass unchanged. If any test fails, the damping path leaked into the
|
||||
undamped trajectory.
|
||||
|
||||
### New tests
|
||||
|
||||
1. **`Gaussian::damp_natural` arithmetic**
|
||||
(`src/gaussian.rs` test mod):
|
||||
- `α = 1.0` returns `new` exactly (bit-equal `pi` and `tau`).
|
||||
- `α = 0.0` returns `self` exactly.
|
||||
- `α = 0.5`: pi and tau are exact midpoints in nat-param space.
|
||||
- Three asserts, no new file.
|
||||
|
||||
2. **`TruncFactor::propagate_with_alpha` shrinks the step**
|
||||
(`src/factor/trunc.rs` test mod):
|
||||
- Set up a TruncFactor step. Run `propagate_with_alpha(α=1.0)` once,
|
||||
record `delta_undamped` and the resulting `self.msg`.
|
||||
- Reset to a fresh factor at the same starting state. Run
|
||||
`propagate_with_alpha(α=0.5)` once, record `delta_damped` and
|
||||
`damped_msg`.
|
||||
- Assert: `damped_msg.pi()` equals `0.5 * undamped_msg.pi() + 0.5 * initial_msg.pi()` within 1e-12 (and same for `tau`).
|
||||
- Assert: `delta_damped.0 <= delta_undamped.0` (mu-delta is no larger; the relationship is monotone in `α` but not strictly `0.5×` for the `delta()` function which is `(|Δmu|, |Δsigma|)`).
|
||||
|
||||
3. **`MarginFactor::propagate_with_alpha` parity**
|
||||
(`src/factor/margin.rs` test mod):
|
||||
- Same shape as #2, on a `MarginFactor` step.
|
||||
|
||||
4. **`run_chain` honours `ConvergenceOptions::max_iter`**
|
||||
(in an existing or new game-level test):
|
||||
- Construct a 4-team ranked game that normally converges in ~5 iterations.
|
||||
- Set `opts.convergence.max_iter = 1`. Assert the per-iteration
|
||||
`step` returned (or observable indirectly via posterior delta vs.
|
||||
the converged answer) is non-zero — i.e. the loop stopped early.
|
||||
- Set `opts.convergence.max_iter = 30`. Assert posteriors match the
|
||||
baseline within `epsilon`.
|
||||
|
||||
5. **Damping default is `1.0` and produces bit-equal output**
|
||||
(smoke test, can be a single assertion in an existing test):
|
||||
- `assert_eq!(ConvergenceOptions::default().alpha, 1.0);`
|
||||
- Existing goldens prove the bit-equality.
|
||||
|
||||
No oscillation-stabilisation test (would require constructing a
|
||||
pathological graph specifically to oscillate; out of scope for a
|
||||
minimal ship).
|
||||
|
||||
## Verification gates
|
||||
|
||||
Per task:
|
||||
|
||||
```bash
|
||||
cargo +nightly fmt
|
||||
cargo clippy --all-targets -- -D warnings
|
||||
cargo test --lib
|
||||
cargo test
|
||||
```
|
||||
|
||||
All must succeed. Test count grows by exactly the new tests above
|
||||
(roughly +5–8 lib tests).
|
||||
|
||||
## Risks
|
||||
|
||||
- **Marginal-update change is subtle.** Switching the variable write
|
||||
from `trunc` to `cavity * damped` is intentionally a no-op when
|
||||
`alpha = 1.0` (since `cavity * new_msg = trunc`), but it changes the
|
||||
arithmetic path. If `Gaussian` arithmetic has any non-associativity
|
||||
in floating-point that the old form happened to dodge, goldens could
|
||||
shift by 1 ULP. Mitigation: TDD — write the regression test (run all
|
||||
existing tests with `alpha = 1.0`) **first**, before changing the
|
||||
variable-write line.
|
||||
- **`run_chain` signature change ripples to two callers.** Trivial
|
||||
but must be done atomically with the field addition on `Game` /
|
||||
`OwnedGame`.
|
||||
- **`alpha` validation only in debug builds.** A release build will
|
||||
silently accept `alpha = 0.0` or `alpha > 1.0` and produce nonsense.
|
||||
This matches the existing pattern (`debug_assert!` for input
|
||||
validation in `Game::ranked_with_arena`); upgrading to `Result` is
|
||||
out of scope.
|
||||
|
||||
## Out-of-scope follow-ups (logged for future plans)
|
||||
|
||||
- Wire `Schedule` into `run_chain` (so `Damped` lands as a real
|
||||
`Schedule` impl alongside `EpsilonOrMax`).
|
||||
- Switch convergence check to `(|Δpi|, |Δtau|)` per spec
|
||||
§"Stopping in natural-param space".
|
||||
- Oscillation auto-detect (engage `alpha < 1.0` only after N
|
||||
non-monotone steps).
|
||||
- `Residual` schedule (priority queue).
|
||||
- `SynergyFactor`, `ScoreFactor` (new EP factor types).
|
||||
@@ -0,0 +1,232 @@
|
||||
# History → TimeSlice ConvergenceOptions Plumbing
|
||||
|
||||
## Summary
|
||||
|
||||
Make `History`'s already-public `ConvergenceOptions` (set via
|
||||
`HistoryBuilder::convergence(...)`) actually reach the within-game
|
||||
inference loop. Today it's read by the outer `History::converge` sweep
|
||||
but dropped on the floor when constructing `TimeSlice`s, so users who
|
||||
opt in to `alpha < 1.0` (Damped EP) on a `History` get nothing — the
|
||||
inner `run_chain` calls inside `TimeSlice` hardcode
|
||||
`ConvergenceOptions::default()`.
|
||||
|
||||
This spec closes the gap with one focused change: thread
|
||||
`ConvergenceOptions` from `History` through `TimeSlice` to the three
|
||||
`Game::*_with_arena` callsites in `time_slice.rs`. No new types, no new
|
||||
public methods on `History` or `HistoryBuilder` — the user-facing API
|
||||
already exists.
|
||||
|
||||
## Background
|
||||
|
||||
After T5 (commit `0705986`) of the Damped EP plan,
|
||||
`Game::*_with_arena` accepts `convergence: ConvergenceOptions` and
|
||||
`run_chain` reads `self.convergence.{epsilon, max_iter, alpha}`.
|
||||
`HistoryBuilder` already has a `convergence(opts)` method (`history.rs:91`)
|
||||
that stores onto a field on `History`. `History::converge` reads
|
||||
`self.convergence.{max_iter, epsilon}` for its outer cross-history loop
|
||||
(`history.rs:437-447`).
|
||||
|
||||
The break is here, in `History::add_events_with_prior` at `history.rs:597`:
|
||||
|
||||
```rust
|
||||
let mut time_slice = TimeSlice::new(t, self.p_draw);
|
||||
```
|
||||
|
||||
`self.convergence` is not passed. `TimeSlice` has no convergence field.
|
||||
The three callsites in `time_slice.rs` that build `Game::*_with_arena`
|
||||
fall back to `ConvergenceOptions::default()`:
|
||||
|
||||
- `Event::iteration_direct` (`time_slice.rs:138-156`)
|
||||
- `TimeSlice::convergence` (`time_slice.rs:332-345`)
|
||||
- `TimeSlice::log_evidence` (`time_slice.rs:521-538`)
|
||||
|
||||
## Scope
|
||||
|
||||
### What ships
|
||||
|
||||
1. `TimeSlice<T>` gains a `pub(crate) convergence: ConvergenceOptions`
|
||||
field set at construction.
|
||||
2. `TimeSlice::new` signature becomes
|
||||
`pub fn new(time: T, p_draw: f64, convergence: ConvergenceOptions) -> Self`.
|
||||
3. `History::add_events_with_prior` (`history.rs:597`) passes
|
||||
`self.convergence` when constructing new `TimeSlice`s.
|
||||
4. `Event::iteration_direct` gains a `convergence: ConvergenceOptions`
|
||||
parameter and forwards it to the `Game::*_with_arena` callsite.
|
||||
The two callers (`TimeSlice::iteration` at `time_slice.rs:419` and
|
||||
`:441`) pass `self.convergence`.
|
||||
5. `TimeSlice::convergence` (the method, not the field) replaces its
|
||||
hardcoded `crate::ConvergenceOptions::default()` with
|
||||
`self.convergence`.
|
||||
6. `TimeSlice::log_evidence` does the same.
|
||||
7. Five test callsites of `TimeSlice::new(time, p_draw)` updated
|
||||
mechanically to `TimeSlice::new(time, p_draw, ConvergenceOptions::default())`.
|
||||
|
||||
### What does not ship
|
||||
|
||||
- No split of `ConvergenceOptions` into outer/inner fields. The
|
||||
conflation (one `max_iter` covers both the cross-history sweep and
|
||||
the per-game EP iteration cap) is the user-confirmed design.
|
||||
- No `Damped` impl in `src/schedule.rs`. The `Schedule` trait is still
|
||||
not integrated into `run_chain`.
|
||||
- No nat-param convergence switch.
|
||||
- No oscillation auto-detect.
|
||||
- No new `History` or `HistoryBuilder` methods. `convergence(opts)`
|
||||
already exists and works.
|
||||
- No changes to `History::converge` — the outer-loop semantics are
|
||||
unchanged (it already reads `self.convergence`).
|
||||
|
||||
## Design
|
||||
|
||||
### `TimeSlice<T>` field
|
||||
|
||||
```rust
|
||||
// src/time_slice.rs
|
||||
pub struct TimeSlice<T: Time = i64> {
|
||||
// ... existing fields ...
|
||||
p_draw: f64,
|
||||
pub(crate) convergence: ConvergenceOptions,
|
||||
// ... existing fields ...
|
||||
}
|
||||
```
|
||||
|
||||
### `TimeSlice::new`
|
||||
|
||||
```rust
|
||||
impl<T: Time> TimeSlice<T> {
|
||||
pub fn new(time: T, p_draw: f64, convergence: ConvergenceOptions) -> Self {
|
||||
Self {
|
||||
// ... existing initialisation ...
|
||||
p_draw,
|
||||
convergence,
|
||||
// ...
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### `History::add_events_with_prior` — single-line fix
|
||||
|
||||
At `src/history.rs:597`:
|
||||
|
||||
```rust
|
||||
// before
|
||||
let mut time_slice = TimeSlice::new(t, self.p_draw);
|
||||
|
||||
// after
|
||||
let mut time_slice = TimeSlice::new(t, self.p_draw, self.convergence);
|
||||
```
|
||||
|
||||
### `Event::iteration_direct` parameter
|
||||
|
||||
```rust
|
||||
// src/time_slice.rs
|
||||
impl Event {
|
||||
pub(crate) fn iteration_direct(
|
||||
&mut self,
|
||||
skills: &mut SkillStore,
|
||||
agents: &CompetitorStore<i64, ConstantDrift>,
|
||||
p_draw: f64,
|
||||
convergence: ConvergenceOptions,
|
||||
arena: &mut ScratchArena,
|
||||
) -> /* existing return */ {
|
||||
// ... existing body, with the Game::*_with_arena calls
|
||||
// using `convergence` instead of ConvergenceOptions::default() ...
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
The two callers — `TimeSlice::iteration` at `time_slice.rs:419` and
|
||||
`:441` — already have `&mut self` access, so they pass
|
||||
`self.convergence`.
|
||||
|
||||
### `TimeSlice::convergence` method (not the field)
|
||||
|
||||
The method `pub(crate) fn convergence<D>(&mut self, agents: ...) -> usize`
|
||||
at `time_slice.rs:447` shares its name with the new field. Rust allows
|
||||
this (methods and fields live in different namespaces), but it's a
|
||||
readability hazard. Rename the method to `iterate_to_convergence` to
|
||||
disambiguate.
|
||||
|
||||
This is one rename, six callsites in `history.rs` and the test module.
|
||||
|
||||
### Field semantics
|
||||
|
||||
`History` keeps the single shared `ConvergenceOptions` struct. The same
|
||||
`max_iter` covers both the outer sweep and each inner per-game loop.
|
||||
The same `epsilon` covers both stopping criteria. The `alpha` field is
|
||||
read only inside `run_chain` (the inner loop); the outer loop
|
||||
intentionally ignores `alpha` because cross-history damping is a
|
||||
different mathematical concept and not in scope.
|
||||
|
||||
## Testing strategy
|
||||
|
||||
### Regression net
|
||||
|
||||
The existing 98 lib + 27 integration tests are the bit-equal regression
|
||||
net. Default `ConvergenceOptions` is unchanged
|
||||
(`max_iter=30, epsilon=1e-6, alpha=1.0`), and `TimeSlice` was already
|
||||
using exactly that since T5. The only behavioural difference is for
|
||||
users who actually pass non-default options through
|
||||
`HistoryBuilder::convergence(...)` — and there are no current tests that
|
||||
do that **and** compare posteriors, so all goldens stay bit-equal.
|
||||
|
||||
### New tests
|
||||
|
||||
1. **`history_propagates_convergence_to_inner_run_chain`** (in
|
||||
`src/history.rs` test module):
|
||||
- Build a History with `convergence(ConvergenceOptions { max_iter: 1, ..Default::default() })`.
|
||||
- Add a small batch of events that needs more than one inner EP iteration to converge (e.g. a 4-team game per slice).
|
||||
- `converge()`, capture posteriors.
|
||||
- Build a fresh History with default options on the same events.
|
||||
- `converge()`, capture posteriors.
|
||||
- Assert the two sets of posteriors differ measurably (max diff > 1e-6).
|
||||
- Proves the inner loop honours the propagated `max_iter`. Today (without this change) the assertion would fail because both Histories use default inside.
|
||||
|
||||
2. **`history_with_damping_reaches_same_fixed_point_as_undamped`** (same
|
||||
test module):
|
||||
- Build a History with `convergence(ConvergenceOptions { alpha: 0.5, max_iter: 200, ..Default::default() })`.
|
||||
- Same events as above.
|
||||
- `converge()`, capture posteriors.
|
||||
- Build a default-options History on the same events.
|
||||
- `converge()`, capture posteriors.
|
||||
- Assert per-player posteriors agree within 1e-3.
|
||||
- Proves damping doesn't break convergence on the History path.
|
||||
|
||||
If the second test's max diff is too large, raise `max_iter` further
|
||||
(damping needs more iterations to reach the same fixed point).
|
||||
|
||||
## Verification gates
|
||||
|
||||
```bash
|
||||
cargo +nightly fmt
|
||||
cargo clippy --all-targets -- -D warnings
|
||||
cargo test --lib
|
||||
cargo test
|
||||
```
|
||||
|
||||
All must succeed. Test count grows by exactly 2 (the two new tests).
|
||||
|
||||
## Risks
|
||||
|
||||
- **`TimeSlice::new` is `pub`.** Adding the third parameter is a
|
||||
breaking change to a public constructor. In a 0.1.x crate this is
|
||||
acceptable, but flag it in the commit message.
|
||||
- **`TimeSlice::convergence` method rename.** Renaming
|
||||
`convergence` → `iterate_to_convergence` touches `history.rs` and the
|
||||
TimeSlice test module. The rename is mechanical and improves
|
||||
readability where the field and method would otherwise share a name.
|
||||
- **Cross-history alpha semantics.** A user who sets `alpha = 0.5` on
|
||||
a `History` gets damping inside every per-game loop, but the outer
|
||||
`History::converge` sweep is undamped. This is the correct semantic
|
||||
(alpha is a within-EP-graph concept) but it's worth documenting in
|
||||
the `ConvergenceOptions::alpha` doc comment so users don't expect
|
||||
cross-slice damping. Add one sentence to the existing doc comment.
|
||||
|
||||
## Out-of-scope follow-ups
|
||||
|
||||
- Wire `Schedule` trait into `run_chain` — Damped becomes a `Schedule`
|
||||
impl alongside `EpsilonOrMax`.
|
||||
- Per-loop `ConvergenceOptions` split (outer / inner).
|
||||
- `Residual` schedule.
|
||||
- Per-event `EventKind::Scored.score_sigma` override (still
|
||||
history-wide today).
|
||||
@@ -0,0 +1,134 @@
|
||||
# Tech Debt Cleanup — Post-T4-MarginFactor
|
||||
|
||||
## Summary
|
||||
|
||||
Three small, independent cleanups left behind by the T4-MarginFactor merge
|
||||
(`8b53cac`). All three are pure code-shape or doc fixes. No public-API change,
|
||||
no numerics change, no new behavior.
|
||||
|
||||
This batch deliberately excludes the `DiffFactor` ↔ `BuiltinFactor` overlap
|
||||
collapse (architectural change kept separate) and per-event `score_sigma`
|
||||
override (a feature, not debt).
|
||||
|
||||
## Scope
|
||||
|
||||
### Item 1 — Deduplicate `Game::likelihoods` and `Game::likelihoods_scored`
|
||||
|
||||
**Current state.** `src/game.rs:236-371` and `src/game.rs:373-485` are 95-line
|
||||
near-duplicates of each other. They differ in exactly one block: the closure
|
||||
that maps a diff index to a `DiffFactor`. The ranked path builds
|
||||
`DiffFactor::Trunc(TruncFactor::new(vid, margin, tie))` with `margin`/`tie`
|
||||
derived from `p_draw` and adjacent-result equality. The scored path builds
|
||||
`DiffFactor::Margin(MarginFactor::new(vid, m_obs, score_sigma))` with `m_obs`
|
||||
the observed score gap. Everything else — sort, `team_prior`, sweep loop,
|
||||
boundary updates, evidence product, posterior `likelihoods` — is bit-identical.
|
||||
|
||||
**Refactor.** Extract a private helper on `OwnedGame<T, D>`:
|
||||
|
||||
```rust
|
||||
fn run_chain<F>(
|
||||
&self,
|
||||
arena: &mut ScratchArena,
|
||||
make_link: F,
|
||||
) -> (f64, Vec<Vec<Gaussian>>)
|
||||
where
|
||||
F: FnMut(usize, &[usize], &mut VarStore) -> DiffFactor,
|
||||
```
|
||||
|
||||
The closure receives the diff index `i`, the descending-by-result sort
|
||||
permutation `&arena.sort_buf`, and `&mut arena.vars` for `alloc(N_INF)`. It
|
||||
returns the `DiffFactor` for that diff slot.
|
||||
|
||||
The helper takes `&self` (not `&mut self`) and returns
|
||||
`(evidence, likelihoods)`. Each caller writes the results back to its own
|
||||
`self.evidence` and `self.likelihoods` fields. The `&self` choice matters: the
|
||||
closure captures `&self.result` / `&self.teams` / `&self.weights` / `&self.p_draw`
|
||||
freely without conflicting with the helper's own immutable borrow.
|
||||
|
||||
The two public methods shrink from ~125 lines each to ~10 lines that just
|
||||
construct the closure.
|
||||
|
||||
**Why a closure (not a trait or two-phase build).** A closure keeps all
|
||||
caller-specific state (`p_draw`, `score_sigma`, beta sums for margin) inline at
|
||||
the call site. A trait would require a stateful object per call; a two-phase
|
||||
build (caller produces the `Vec<DiffFactor>` first, helper does the rest) would
|
||||
either re-do the sort or split state ownership awkwardly between phases.
|
||||
|
||||
### Item 2 — Make `BuiltinFactor::log_evidence` exhaustive
|
||||
|
||||
**Current state.** `src/factor/mod.rs:94-100` uses a `_ => 0.0` wildcard for
|
||||
`TeamSum` and `RankDiff`. When a future variant lands (e.g. `SynergyFactor`),
|
||||
the wildcard silently absorbs it instead of forcing a deliberate decision.
|
||||
|
||||
**Refactor.**
|
||||
|
||||
```rust
|
||||
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,
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
No behavioral change. Future variants now produce a non-exhaustive-match
|
||||
compile error.
|
||||
|
||||
### Item 3 — Fix stale numerics in T4 plan doc
|
||||
|
||||
**Current state.** `docs/superpowers/plans/2026-04-27-t4-margin-factor.md`
|
||||
contains two numbers that diverge from the values asserted by the shipped test
|
||||
in `src/factor/mod.rs:163,166`.
|
||||
|
||||
**Fix.**
|
||||
|
||||
| Doc value (wrong) | Implementation value (correct) |
|
||||
|---|---|
|
||||
| `0.046827` | `0.04678` |
|
||||
| `-3.0613` | `-3.0622` |
|
||||
|
||||
Pure docs change. Verified by reading the asserted constants in the test.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- **`DiffFactor` ↔ `BuiltinFactor` overlap.** Both enums list `Trunc` and
|
||||
`Margin` variants. Collapsing into `BuiltinFactor::Diff(DiffFactor)` is
|
||||
defensible but is an architectural change that wants its own design pass.
|
||||
`DiffFactor` represents a real semantic subset (factors that operate on a
|
||||
diff variable in a chain); the duplication is two enum variants, not a
|
||||
large block of code.
|
||||
- **Per-event `EventKind::Scored.score_sigma` override.** Today
|
||||
`score_sigma` is history-wide (set on `HistoryBuilder::score_sigma`). A
|
||||
per-event override is a real feature ask, not tech debt.
|
||||
|
||||
## Verification
|
||||
|
||||
Each item commits independently and ships behind a green `cargo test --lib`
|
||||
run. The dedup is a pure code-shape change: posteriors and evidence must be
|
||||
**bit-equal** (not ULP-bounded) against the existing 88+28 test goldens.
|
||||
|
||||
Per-item gate before committing:
|
||||
|
||||
```bash
|
||||
cargo +nightly fmt
|
||||
cargo clippy
|
||||
cargo test --lib
|
||||
```
|
||||
|
||||
## Commit shape
|
||||
|
||||
Three commits, one per item, each independently revertable:
|
||||
|
||||
1. `refactor: dedupe Game::likelihoods and likelihoods_scored via run_chain`
|
||||
2. `refactor: make BuiltinFactor::log_evidence match exhaustive`
|
||||
3. `docs: fix stale numerics in t4-margin-factor plan`
|
||||
|
||||
## Risks
|
||||
|
||||
- **Borrow-checker friction in Item 1.** The closure captures fields of
|
||||
`&self` while the helper iterates over arena state. Mitigation: helper is
|
||||
`&self` (not `&mut self`); arena passed as `&mut ScratchArena` separately.
|
||||
Disjoint borrows.
|
||||
- **Compile error in Item 2 if a new variant ships before this lands.**
|
||||
Trivial follow-on; the whole point is to surface that signal.
|
||||
@@ -48,6 +48,7 @@ fn main() {
|
||||
.convergence(trueskill_tt::ConvergenceOptions {
|
||||
max_iter: 10,
|
||||
epsilon: 0.01,
|
||||
alpha: 1.0,
|
||||
})
|
||||
.build();
|
||||
|
||||
|
||||
+1
-1
@@ -1,2 +1,2 @@
|
||||
publish = false
|
||||
pre-release-hook = ["sh", "-c", "git cliff -o ../CHANGELOG.md --tag {{version}} && git add CHANGELOG.md"]
|
||||
pre-release-hook = ["sh", "-c", "git cliff -o CHANGELOG.md --tag {{version}} && git add CHANGELOG.md"]
|
||||
|
||||
@@ -8,6 +8,16 @@ use smallvec::SmallVec;
|
||||
pub struct ConvergenceOptions {
|
||||
pub max_iter: usize,
|
||||
pub epsilon: f64,
|
||||
/// EP damping factor in natural-parameter space: each per-factor
|
||||
/// update inside a single game writes `α·new + (1−α)·old`. `1.0` is
|
||||
/// undamped (default); `< 1.0` stabilises oscillating fixed-point
|
||||
/// loops at the cost of more iterations. Must be in `(0.0, 1.0]`.
|
||||
///
|
||||
/// Applies only to the within-game EP loop (`run_chain`). The outer
|
||||
/// `History::converge` cross-history sweep is undamped regardless of
|
||||
/// this value — cross-slice damping is a different concept and not
|
||||
/// in scope.
|
||||
pub alpha: f64,
|
||||
}
|
||||
|
||||
impl Default for ConvergenceOptions {
|
||||
@@ -15,6 +25,7 @@ impl Default for ConvergenceOptions {
|
||||
Self {
|
||||
max_iter: crate::ITERATIONS,
|
||||
epsilon: crate::EPSILON,
|
||||
alpha: 1.0,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -29,3 +40,14 @@ pub struct ConvergenceReport {
|
||||
pub per_iteration_time: SmallVec<[Duration; 32]>,
|
||||
pub slices_skipped: usize,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn default_alpha_is_one_for_undamped_behavior() {
|
||||
let opts = ConvergenceOptions::default();
|
||||
assert_eq!(opts.alpha, 1.0);
|
||||
}
|
||||
}
|
||||
|
||||
+60
-6
@@ -32,8 +32,11 @@ impl MarginFactor {
|
||||
}
|
||||
}
|
||||
|
||||
impl Factor for MarginFactor {
|
||||
fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64) {
|
||||
impl MarginFactor {
|
||||
/// Propagate this factor's message, optionally damping the update in
|
||||
/// natural-parameter space. `alpha = 1.0` matches `Factor::propagate`
|
||||
/// exactly; `alpha < 1.0` writes `α·new_msg + (1−α)·old_msg`.
|
||||
pub(crate) fn propagate_with_alpha(&mut self, vars: &mut VarStore, alpha: f64) -> (f64, f64) {
|
||||
let marginal = vars.get(self.diff);
|
||||
let cavity = marginal / self.msg;
|
||||
|
||||
@@ -42,12 +45,18 @@ impl Factor for MarginFactor {
|
||||
}
|
||||
|
||||
let new_msg = Gaussian::from_ms(self.m_obs, self.sigma);
|
||||
let new_marginal = cavity * new_msg;
|
||||
let damped = self.msg.damp_natural(new_msg, alpha);
|
||||
let old_msg = self.msg;
|
||||
self.msg = new_msg;
|
||||
vars.set(self.diff, new_marginal);
|
||||
self.msg = damped;
|
||||
vars.set(self.diff, cavity * damped);
|
||||
|
||||
old_msg.delta(new_msg)
|
||||
old_msg.delta(damped)
|
||||
}
|
||||
}
|
||||
|
||||
impl Factor for MarginFactor {
|
||||
fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64) {
|
||||
self.propagate_with_alpha(vars, 1.0)
|
||||
}
|
||||
|
||||
fn log_evidence(&self, _vars: &VarStore) -> f64 {
|
||||
@@ -120,4 +129,49 @@ mod tests {
|
||||
let logz = f.log_evidence(&vars);
|
||||
assert!((logz - (-3.062235327364623)).abs() < 1e-10);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn propagate_with_alpha_one_matches_undamped_propagate() {
|
||||
let mut vars_a = VarStore::new();
|
||||
let diff_a = vars_a.alloc(Gaussian::from_ms(0.0, 6.0));
|
||||
let mut f_a = MarginFactor::new(diff_a, 5.0, 1.0);
|
||||
let delta_a = f_a.propagate(&mut vars_a);
|
||||
let result_a = vars_a.get(diff_a);
|
||||
|
||||
let mut vars_b = VarStore::new();
|
||||
let diff_b = vars_b.alloc(Gaussian::from_ms(0.0, 6.0));
|
||||
let mut f_b = MarginFactor::new(diff_b, 5.0, 1.0);
|
||||
let delta_b = f_b.propagate_with_alpha(&mut vars_b, 1.0);
|
||||
let result_b = vars_b.get(diff_b);
|
||||
|
||||
assert_eq!(result_a.pi(), result_b.pi());
|
||||
assert_eq!(result_a.tau(), result_b.tau());
|
||||
assert_eq!(delta_a, delta_b);
|
||||
assert_eq!(f_a.msg.pi(), f_b.msg.pi());
|
||||
assert_eq!(f_a.msg.tau(), f_b.msg.tau());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn propagate_with_alpha_half_blends_msg_in_natural_params() {
|
||||
// Run undamped to capture (initial_msg, undamped_new_msg).
|
||||
let mut vars_full = VarStore::new();
|
||||
let diff_full = vars_full.alloc(Gaussian::from_ms(0.0, 6.0));
|
||||
let mut f_full = MarginFactor::new(diff_full, 5.0, 1.0);
|
||||
let initial_msg_pi = f_full.msg.pi();
|
||||
let initial_msg_tau = f_full.msg.tau();
|
||||
f_full.propagate(&mut vars_full);
|
||||
let undamped_msg_pi = f_full.msg.pi();
|
||||
let undamped_msg_tau = f_full.msg.tau();
|
||||
|
||||
// Run damped at α = 0.5 from the same initial state.
|
||||
let mut vars_half = VarStore::new();
|
||||
let diff_half = vars_half.alloc(Gaussian::from_ms(0.0, 6.0));
|
||||
let mut f_half = MarginFactor::new(diff_half, 5.0, 1.0);
|
||||
f_half.propagate_with_alpha(&mut vars_half, 0.5);
|
||||
|
||||
let expected_pi = 0.5 * undamped_msg_pi + 0.5 * initial_msg_pi;
|
||||
let expected_tau = 0.5 * undamped_msg_tau + 0.5 * initial_msg_tau;
|
||||
assert!((f_half.msg.pi() - expected_pi).abs() < 1e-12);
|
||||
assert!((f_half.msg.tau() - expected_tau).abs() < 1e-12);
|
||||
}
|
||||
}
|
||||
|
||||
+1
-1
@@ -95,7 +95,7 @@ impl Factor for BuiltinFactor {
|
||||
match self {
|
||||
Self::Trunc(f) => f.log_evidence(vars),
|
||||
Self::Margin(f) => f.log_evidence(vars),
|
||||
_ => 0.0,
|
||||
Self::TeamSum(_) | Self::RankDiff(_) => 0.0,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+64
-11
@@ -33,29 +33,37 @@ impl TruncFactor {
|
||||
}
|
||||
}
|
||||
|
||||
impl Factor for TruncFactor {
|
||||
fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64) {
|
||||
impl TruncFactor {
|
||||
/// Propagate this factor's message, optionally damping the update in
|
||||
/// natural-parameter space. `alpha = 1.0` matches `Factor::propagate`
|
||||
/// exactly; `alpha < 1.0` writes `α·new_msg + (1−α)·old_msg`.
|
||||
pub(crate) fn propagate_with_alpha(&mut self, vars: &mut VarStore, alpha: f64) -> (f64, f64) {
|
||||
let marginal = vars.get(self.diff);
|
||||
// Cavity: marginal divided by our outgoing message.
|
||||
let cavity = marginal / self.msg;
|
||||
|
||||
// First-time-only: cache the evidence contribution from the cavity.
|
||||
if self.evidence_cached.is_none() {
|
||||
self.evidence_cached = Some(cavity_evidence(cavity, self.margin, self.tie));
|
||||
}
|
||||
|
||||
// Apply the truncation approximation to the cavity.
|
||||
let trunc = approx(cavity, self.margin, self.tie);
|
||||
|
||||
// New outgoing message such that cavity * new_msg = trunc.
|
||||
let new_msg = trunc / cavity;
|
||||
|
||||
let damped = self.msg.damp_natural(new_msg, alpha);
|
||||
let old_msg = self.msg;
|
||||
self.msg = new_msg;
|
||||
self.msg = damped;
|
||||
|
||||
// Update the marginal: marginal_new = cavity * new_msg = trunc.
|
||||
vars.set(self.diff, trunc);
|
||||
// marginal_new = cavity * stored_msg. With alpha = 1.0 this equals
|
||||
// `trunc` (since cavity * new_msg = trunc by construction); with
|
||||
// alpha < 1.0 it reflects the partially-applied update.
|
||||
vars.set(self.diff, cavity * damped);
|
||||
|
||||
old_msg.delta(new_msg)
|
||||
old_msg.delta(damped)
|
||||
}
|
||||
}
|
||||
|
||||
impl Factor for TruncFactor {
|
||||
fn propagate(&mut self, vars: &mut VarStore) -> (f64, f64) {
|
||||
self.propagate_with_alpha(vars, 1.0)
|
||||
}
|
||||
|
||||
fn log_evidence(&self, _vars: &VarStore) -> f64 {
|
||||
@@ -127,4 +135,49 @@ mod tests {
|
||||
let ev = f.evidence_cached.unwrap();
|
||||
assert!(ev > 0.35 && ev < 0.42);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn propagate_with_alpha_one_matches_undamped_propagate() {
|
||||
let mut vars_a = VarStore::new();
|
||||
let diff_a = vars_a.alloc(Gaussian::from_ms(2.0, 3.0));
|
||||
let mut f_a = TruncFactor::new(diff_a, 0.0, false);
|
||||
let delta_a = f_a.propagate(&mut vars_a);
|
||||
let result_a = vars_a.get(diff_a);
|
||||
|
||||
let mut vars_b = VarStore::new();
|
||||
let diff_b = vars_b.alloc(Gaussian::from_ms(2.0, 3.0));
|
||||
let mut f_b = TruncFactor::new(diff_b, 0.0, false);
|
||||
let delta_b = f_b.propagate_with_alpha(&mut vars_b, 1.0);
|
||||
let result_b = vars_b.get(diff_b);
|
||||
|
||||
assert_eq!(result_a.pi(), result_b.pi());
|
||||
assert_eq!(result_a.tau(), result_b.tau());
|
||||
assert_eq!(delta_a, delta_b);
|
||||
assert_eq!(f_a.msg.pi(), f_b.msg.pi());
|
||||
assert_eq!(f_a.msg.tau(), f_b.msg.tau());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn propagate_with_alpha_half_blends_msg_in_natural_params() {
|
||||
// Run undamped to capture (initial_msg, undamped_new_msg).
|
||||
let mut vars_full = VarStore::new();
|
||||
let diff_full = vars_full.alloc(Gaussian::from_ms(2.0, 3.0));
|
||||
let mut f_full = TruncFactor::new(diff_full, 0.0, false);
|
||||
let initial_msg_pi = f_full.msg.pi();
|
||||
let initial_msg_tau = f_full.msg.tau();
|
||||
f_full.propagate(&mut vars_full);
|
||||
let undamped_msg_pi = f_full.msg.pi();
|
||||
let undamped_msg_tau = f_full.msg.tau();
|
||||
|
||||
// Run damped at α = 0.5 from the same initial state.
|
||||
let mut vars_half = VarStore::new();
|
||||
let diff_half = vars_half.alloc(Gaussian::from_ms(2.0, 3.0));
|
||||
let mut f_half = TruncFactor::new(diff_half, 0.0, false);
|
||||
f_half.propagate_with_alpha(&mut vars_half, 0.5);
|
||||
|
||||
let expected_pi = 0.5 * undamped_msg_pi + 0.5 * initial_msg_pi;
|
||||
let expected_tau = 0.5 * undamped_msg_tau + 0.5 * initial_msg_tau;
|
||||
assert!((f_half.msg.pi() - expected_pi).abs() < 1e-12);
|
||||
assert!((f_half.msg.tau() - expected_tau).abs() < 1e-12);
|
||||
}
|
||||
}
|
||||
|
||||
+224
-155
@@ -44,11 +44,14 @@ impl DiffFactor {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn propagate(&mut self, vars: &mut crate::factor::VarStore) -> (f64, f64) {
|
||||
use crate::factor::Factor;
|
||||
pub(crate) fn propagate(
|
||||
&mut self,
|
||||
vars: &mut crate::factor::VarStore,
|
||||
alpha: f64,
|
||||
) -> (f64, f64) {
|
||||
match self {
|
||||
Self::Trunc(f) => f.propagate(vars),
|
||||
Self::Margin(f) => f.propagate(vars),
|
||||
Self::Trunc(f) => f.propagate_with_alpha(vars, alpha),
|
||||
Self::Margin(f) => f.propagate_with_alpha(vars, alpha),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -87,6 +90,7 @@ pub struct OwnedGame<T: Time, D: Drift<T>> {
|
||||
result: Vec<f64>,
|
||||
weights: Vec<Vec<f64>>,
|
||||
p_draw: f64,
|
||||
pub(crate) convergence: crate::ConvergenceOptions,
|
||||
pub(crate) likelihoods: Vec<Vec<Gaussian>>,
|
||||
pub(crate) evidence: f64,
|
||||
}
|
||||
@@ -97,9 +101,17 @@ impl<T: Time, D: Drift<T>> OwnedGame<T, D> {
|
||||
result: Vec<f64>,
|
||||
weights: Vec<Vec<f64>>,
|
||||
p_draw: f64,
|
||||
convergence: crate::ConvergenceOptions,
|
||||
) -> Self {
|
||||
let mut arena = ScratchArena::new();
|
||||
let g = Game::ranked_with_arena(teams.clone(), &result, &weights, p_draw, &mut arena);
|
||||
let g = Game::ranked_with_arena(
|
||||
teams.clone(),
|
||||
&result,
|
||||
&weights,
|
||||
p_draw,
|
||||
convergence,
|
||||
&mut arena,
|
||||
);
|
||||
let likelihoods = g.likelihoods;
|
||||
let evidence = g.evidence;
|
||||
Self {
|
||||
@@ -107,6 +119,7 @@ impl<T: Time, D: Drift<T>> OwnedGame<T, D> {
|
||||
result,
|
||||
weights,
|
||||
p_draw,
|
||||
convergence,
|
||||
likelihoods,
|
||||
evidence,
|
||||
}
|
||||
@@ -117,9 +130,17 @@ impl<T: Time, D: Drift<T>> OwnedGame<T, D> {
|
||||
scores: Vec<f64>,
|
||||
weights: Vec<Vec<f64>>,
|
||||
score_sigma: f64,
|
||||
convergence: crate::ConvergenceOptions,
|
||||
) -> Self {
|
||||
let mut arena = ScratchArena::new();
|
||||
let g = Game::scored_with_arena(teams.clone(), &scores, &weights, score_sigma, &mut arena);
|
||||
let g = Game::scored_with_arena(
|
||||
teams.clone(),
|
||||
&scores,
|
||||
&weights,
|
||||
score_sigma,
|
||||
convergence,
|
||||
&mut arena,
|
||||
);
|
||||
let likelihoods = g.likelihoods;
|
||||
let evidence = g.evidence;
|
||||
Self {
|
||||
@@ -127,6 +148,7 @@ impl<T: Time, D: Drift<T>> OwnedGame<T, D> {
|
||||
result: scores,
|
||||
weights,
|
||||
p_draw: 0.0,
|
||||
convergence,
|
||||
likelihoods,
|
||||
evidence,
|
||||
}
|
||||
@@ -151,6 +173,7 @@ pub struct Game<'a, T: Time = i64, D: Drift<T> = crate::drift::ConstantDrift> {
|
||||
result: &'a [f64],
|
||||
weights: &'a [Vec<f64>],
|
||||
p_draw: f64,
|
||||
pub(crate) convergence: crate::ConvergenceOptions,
|
||||
pub(crate) likelihoods: Vec<Vec<Gaussian>>,
|
||||
pub(crate) evidence: f64,
|
||||
}
|
||||
@@ -161,6 +184,7 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
|
||||
result: &'a [f64],
|
||||
weights: &'a [Vec<f64>],
|
||||
p_draw: f64,
|
||||
convergence: crate::ConvergenceOptions,
|
||||
arena: &mut ScratchArena,
|
||||
) -> Self {
|
||||
debug_assert!(
|
||||
@@ -186,12 +210,17 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
|
||||
},
|
||||
"draw must be > 0.0 if there are teams with draw"
|
||||
);
|
||||
debug_assert!(
|
||||
convergence.alpha > 0.0 && convergence.alpha <= 1.0,
|
||||
"convergence alpha must be in (0.0, 1.0]"
|
||||
);
|
||||
|
||||
let mut this = Self {
|
||||
teams,
|
||||
result,
|
||||
weights,
|
||||
p_draw,
|
||||
convergence,
|
||||
likelihoods: Vec::new(),
|
||||
evidence: 0.0,
|
||||
};
|
||||
@@ -205,6 +234,7 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
|
||||
scores: &'a [f64],
|
||||
weights: &'a [Vec<f64>],
|
||||
score_sigma: f64,
|
||||
convergence: crate::ConvergenceOptions,
|
||||
arena: &mut ScratchArena,
|
||||
) -> Self {
|
||||
debug_assert!(
|
||||
@@ -219,12 +249,17 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
|
||||
"weights must have the same dimensions as teams"
|
||||
);
|
||||
debug_assert!(score_sigma > 0.0, "score_sigma must be positive");
|
||||
debug_assert!(
|
||||
convergence.alpha > 0.0 && convergence.alpha <= 1.0,
|
||||
"convergence alpha must be in (0.0, 1.0]"
|
||||
);
|
||||
|
||||
let mut this = Self {
|
||||
teams,
|
||||
result: scores,
|
||||
weights,
|
||||
p_draw: 0.0,
|
||||
convergence,
|
||||
likelihoods: Vec::new(),
|
||||
evidence: 0.0,
|
||||
};
|
||||
@@ -233,12 +268,18 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
|
||||
this
|
||||
}
|
||||
|
||||
fn likelihoods(&mut self, arena: &mut ScratchArena) {
|
||||
fn run_chain<F>(&self, arena: &mut ScratchArena, mut make_link: F) -> (f64, Vec<Vec<Gaussian>>)
|
||||
where
|
||||
F: FnMut(usize, &[usize], &mut crate::factor::VarStore) -> DiffFactor,
|
||||
{
|
||||
arena.reset();
|
||||
|
||||
let alpha = self.convergence.alpha;
|
||||
let epsilon = self.convergence.epsilon;
|
||||
let max_iter = self.convergence.max_iter;
|
||||
|
||||
let n_teams = self.teams.len();
|
||||
|
||||
// Sort teams by result descending; reuse arena.sort_buf to avoid allocation.
|
||||
arena.sort_buf.extend(0..n_teams);
|
||||
arena.sort_buf.sort_by(|&i, &j| {
|
||||
self.result[j]
|
||||
@@ -246,7 +287,6 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
|
||||
.unwrap_or(Ordering::Equal)
|
||||
});
|
||||
|
||||
// Team performance priors written into arena buffer (capacity reused across games).
|
||||
arena.team_prior.extend(arena.sort_buf.iter().map(|&t| {
|
||||
self.teams[t]
|
||||
.iter()
|
||||
@@ -256,46 +296,25 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
|
||||
|
||||
let n_diffs = n_teams.saturating_sub(1);
|
||||
|
||||
// One DiffFactor per adjacent sorted-team pair; each owns a diff VarId.
|
||||
// links stays local (fresh state per game; Vec capacity is typically small).
|
||||
let mut links: Vec<DiffFactor> = (0..n_diffs)
|
||||
.map(|i| {
|
||||
let tie = self.result[arena.sort_buf[i]] == self.result[arena.sort_buf[i + 1]];
|
||||
let margin = if self.p_draw == 0.0 {
|
||||
0.0
|
||||
} else {
|
||||
let a: f64 = self.teams[arena.sort_buf[i]]
|
||||
.iter()
|
||||
.map(|p| p.beta.powi(2))
|
||||
.sum();
|
||||
let b: f64 = self.teams[arena.sort_buf[i + 1]]
|
||||
.iter()
|
||||
.map(|p| p.beta.powi(2))
|
||||
.sum();
|
||||
compute_margin(self.p_draw, (a + b).sqrt())
|
||||
};
|
||||
let vid = arena.vars.alloc(N_INF);
|
||||
DiffFactor::Trunc(TruncFactor::new(vid, margin, tie))
|
||||
})
|
||||
.map(|i| make_link(i, &arena.sort_buf, &mut arena.vars))
|
||||
.collect();
|
||||
|
||||
// Per-team messages from neighbouring RankDiff factors (replaces TeamMessage).
|
||||
arena.lhood_lose.resize(n_teams, N_INF);
|
||||
arena.lhood_win.resize(n_teams, N_INF);
|
||||
|
||||
let mut step = (f64::INFINITY, f64::INFINITY);
|
||||
let mut iter = 0;
|
||||
|
||||
while tuple_gt(step, 1e-6) && iter < 10 {
|
||||
while tuple_gt(step, epsilon) && iter < max_iter {
|
||||
step = (0.0_f64, 0.0_f64);
|
||||
|
||||
// Forward sweep: diffs 0 .. n_diffs-2 (all but the last).
|
||||
for (e, lf) in links[..n_diffs.saturating_sub(1)].iter_mut().enumerate() {
|
||||
let pw = arena.team_prior[e] * arena.lhood_lose[e];
|
||||
let pl = arena.team_prior[e + 1] * arena.lhood_win[e + 1];
|
||||
let raw = pw - pl;
|
||||
arena.vars.set(lf.diff(), raw * lf.msg());
|
||||
let d = lf.propagate(&mut arena.vars);
|
||||
let d = lf.propagate(&mut arena.vars, alpha);
|
||||
step = tuple_max(step, d);
|
||||
|
||||
let new_ll = pw - lf.msg();
|
||||
@@ -303,14 +322,13 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
|
||||
arena.lhood_lose[e + 1] = new_ll;
|
||||
}
|
||||
|
||||
// Backward sweep: diffs n_diffs-1 .. 1 (reverse, all but the first).
|
||||
for (rev_i, lf) in links[1..].iter_mut().rev().enumerate() {
|
||||
let e = n_diffs - 1 - rev_i;
|
||||
let pw = arena.team_prior[e] * arena.lhood_lose[e];
|
||||
let pl = arena.team_prior[e + 1] * arena.lhood_win[e + 1];
|
||||
let raw = pw - pl;
|
||||
arena.vars.set(lf.diff(), raw * lf.msg());
|
||||
let d = lf.propagate(&mut arena.vars);
|
||||
let d = lf.propagate(&mut arena.vars, alpha);
|
||||
step = tuple_max(step, d);
|
||||
|
||||
let new_lw = pl + lf.msg();
|
||||
@@ -326,7 +344,7 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
|
||||
let raw = (arena.team_prior[0] * arena.lhood_lose[0])
|
||||
- (arena.team_prior[1] * arena.lhood_win[1]);
|
||||
arena.vars.set(links[0].diff(), raw * links[0].msg());
|
||||
links[0].propagate(&mut arena.vars);
|
||||
links[0].propagate(&mut arena.vars, alpha);
|
||||
}
|
||||
|
||||
// Boundary updates: close the chain at both ends.
|
||||
@@ -337,8 +355,7 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
|
||||
arena.lhood_lose[n_teams - 1] = pw_last - links[n_diffs - 1].msg();
|
||||
}
|
||||
|
||||
// Evidence = product of per-diff evidences (each cached on first propagation).
|
||||
self.evidence = links.iter().map(|l| l.evidence()).product();
|
||||
let evidence: f64 = links.iter().map(|l| l.evidence()).product();
|
||||
|
||||
// Inverse permutation: inv_buf[orig_i] = sorted_i.
|
||||
arena.inv_buf.resize(n_teams, 0);
|
||||
@@ -346,7 +363,7 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
|
||||
arena.inv_buf[orig_i] = si;
|
||||
}
|
||||
|
||||
self.likelihoods = self
|
||||
let likelihoods = self
|
||||
.teams
|
||||
.iter()
|
||||
.zip(self.weights.iter())
|
||||
@@ -368,120 +385,38 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
|
||||
.collect::<Vec<_>>()
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
(evidence, likelihoods)
|
||||
}
|
||||
|
||||
fn likelihoods(&mut self, arena: &mut ScratchArena) {
|
||||
let (evidence, likelihoods) = self.run_chain(arena, |i, sort_buf, vars| {
|
||||
let tie = self.result[sort_buf[i]] == self.result[sort_buf[i + 1]];
|
||||
let margin = if self.p_draw == 0.0 {
|
||||
0.0
|
||||
} else {
|
||||
let a: f64 = self.teams[sort_buf[i]].iter().map(|p| p.beta.powi(2)).sum();
|
||||
let b: f64 = self.teams[sort_buf[i + 1]]
|
||||
.iter()
|
||||
.map(|p| p.beta.powi(2))
|
||||
.sum();
|
||||
compute_margin(self.p_draw, (a + b).sqrt())
|
||||
};
|
||||
let vid = vars.alloc(N_INF);
|
||||
DiffFactor::Trunc(TruncFactor::new(vid, margin, tie))
|
||||
});
|
||||
self.evidence = evidence;
|
||||
self.likelihoods = likelihoods;
|
||||
}
|
||||
|
||||
fn likelihoods_scored(&mut self, arena: &mut ScratchArena, score_sigma: f64) {
|
||||
arena.reset();
|
||||
|
||||
let n_teams = self.teams.len();
|
||||
|
||||
arena.sort_buf.extend(0..n_teams);
|
||||
arena.sort_buf.sort_by(|&i, &j| {
|
||||
self.result[j]
|
||||
.partial_cmp(&self.result[i])
|
||||
.unwrap_or(Ordering::Equal)
|
||||
});
|
||||
|
||||
arena.team_prior.extend(arena.sort_buf.iter().map(|&t| {
|
||||
self.teams[t]
|
||||
.iter()
|
||||
.zip(self.weights[t].iter())
|
||||
.fold(N00, |p, (player, &w)| p + (player.performance() * w))
|
||||
}));
|
||||
|
||||
let n_diffs = n_teams.saturating_sub(1);
|
||||
|
||||
let mut links: Vec<DiffFactor> = (0..n_diffs)
|
||||
.map(|i| {
|
||||
// After descending-by-score sort, m_obs >= 0 for every adjacent pair.
|
||||
let m_obs = self.result[arena.sort_buf[i]] - self.result[arena.sort_buf[i + 1]];
|
||||
let vid = arena.vars.alloc(N_INF);
|
||||
let (evidence, likelihoods) = self.run_chain(arena, |i, sort_buf, vars| {
|
||||
let m_obs = self.result[sort_buf[i]] - self.result[sort_buf[i + 1]];
|
||||
let vid = vars.alloc(N_INF);
|
||||
DiffFactor::Margin(MarginFactor::new(vid, m_obs, score_sigma))
|
||||
})
|
||||
.collect();
|
||||
|
||||
arena.lhood_lose.resize(n_teams, N_INF);
|
||||
arena.lhood_win.resize(n_teams, N_INF);
|
||||
|
||||
let mut step = (f64::INFINITY, f64::INFINITY);
|
||||
let mut iter = 0;
|
||||
|
||||
while tuple_gt(step, 1e-6) && iter < 10 {
|
||||
step = (0.0_f64, 0.0_f64);
|
||||
|
||||
for (e, lf) in links[..n_diffs.saturating_sub(1)].iter_mut().enumerate() {
|
||||
let pw = arena.team_prior[e] * arena.lhood_lose[e];
|
||||
let pl = arena.team_prior[e + 1] * arena.lhood_win[e + 1];
|
||||
let raw = pw - pl;
|
||||
arena.vars.set(lf.diff(), raw * lf.msg());
|
||||
let d = lf.propagate(&mut arena.vars);
|
||||
step = tuple_max(step, d);
|
||||
|
||||
let new_ll = pw - lf.msg();
|
||||
step = tuple_max(step, arena.lhood_lose[e + 1].delta(new_ll));
|
||||
arena.lhood_lose[e + 1] = new_ll;
|
||||
}
|
||||
|
||||
for (rev_i, lf) in links[1..].iter_mut().rev().enumerate() {
|
||||
let e = n_diffs - 1 - rev_i;
|
||||
let pw = arena.team_prior[e] * arena.lhood_lose[e];
|
||||
let pl = arena.team_prior[e + 1] * arena.lhood_win[e + 1];
|
||||
let raw = pw - pl;
|
||||
arena.vars.set(lf.diff(), raw * lf.msg());
|
||||
let d = lf.propagate(&mut arena.vars);
|
||||
step = tuple_max(step, d);
|
||||
|
||||
let new_lw = pl + lf.msg();
|
||||
step = tuple_max(step, arena.lhood_win[e].delta(new_lw));
|
||||
arena.lhood_win[e] = new_lw;
|
||||
}
|
||||
|
||||
iter += 1;
|
||||
}
|
||||
|
||||
if n_diffs == 1 {
|
||||
let raw = (arena.team_prior[0] * arena.lhood_lose[0])
|
||||
- (arena.team_prior[1] * arena.lhood_win[1]);
|
||||
arena.vars.set(links[0].diff(), raw * links[0].msg());
|
||||
links[0].propagate(&mut arena.vars);
|
||||
}
|
||||
|
||||
if n_diffs > 0 {
|
||||
let pl1 = arena.team_prior[1] * arena.lhood_win[1];
|
||||
arena.lhood_win[0] = pl1 + links[0].msg();
|
||||
let pw_last = arena.team_prior[n_teams - 2] * arena.lhood_lose[n_teams - 2];
|
||||
arena.lhood_lose[n_teams - 1] = pw_last - links[n_diffs - 1].msg();
|
||||
}
|
||||
|
||||
self.evidence = links.iter().map(|l| l.evidence()).product();
|
||||
|
||||
arena.inv_buf.resize(n_teams, 0);
|
||||
for (si, &orig_i) in arena.sort_buf.iter().enumerate() {
|
||||
arena.inv_buf[orig_i] = si;
|
||||
}
|
||||
|
||||
self.likelihoods = self
|
||||
.teams
|
||||
.iter()
|
||||
.zip(self.weights.iter())
|
||||
.enumerate()
|
||||
.map(|(orig_i, (players, weights))| {
|
||||
let si = arena.inv_buf[orig_i];
|
||||
let m = arena.lhood_win[si] * arena.lhood_lose[si];
|
||||
let performance = players
|
||||
.iter()
|
||||
.zip(weights.iter())
|
||||
.fold(N00, |p, (player, &w)| p + (player.performance() * w));
|
||||
players
|
||||
.iter()
|
||||
.zip(weights.iter())
|
||||
.map(|(player, &w)| {
|
||||
((m - performance.exclude(player.performance() * w)) * (1.0 / w))
|
||||
.forget(player.beta.powi(2))
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
});
|
||||
self.evidence = evidence;
|
||||
self.likelihoods = likelihoods;
|
||||
}
|
||||
|
||||
pub fn posteriors(&self) -> Vec<Vec<Gaussian>> {
|
||||
@@ -533,7 +468,13 @@ impl<T: Time, D: Drift<T>> Game<'_, T, D> {
|
||||
let teams_owned: Vec<Vec<Rating<T, D>>> = teams.iter().map(|t| t.to_vec()).collect();
|
||||
let weights: Vec<Vec<f64>> = teams.iter().map(|t| vec![1.0; t.len()]).collect();
|
||||
|
||||
Ok(OwnedGame::new(teams_owned, result, weights, options.p_draw))
|
||||
Ok(OwnedGame::new(
|
||||
teams_owned,
|
||||
result,
|
||||
weights,
|
||||
options.p_draw,
|
||||
options.convergence,
|
||||
))
|
||||
}
|
||||
|
||||
pub fn scored(
|
||||
@@ -569,6 +510,7 @@ impl<T: Time, D: Drift<T>> Game<'_, T, D> {
|
||||
scores,
|
||||
weights,
|
||||
options.score_sigma,
|
||||
options.convergence,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -630,6 +572,7 @@ mod tests {
|
||||
&[0.0, 1.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -657,6 +600,7 @@ mod tests {
|
||||
&[0.0, 1.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -676,6 +620,7 @@ mod tests {
|
||||
&[0.0, 1.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
|
||||
@@ -709,6 +654,7 @@ mod tests {
|
||||
&[1.0, 2.0, 0.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -725,6 +671,7 @@ mod tests {
|
||||
&[2.0, 1.0, 0.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -736,7 +683,14 @@ mod tests {
|
||||
assert_ulps_eq!(b, Gaussian::from_ms(25.000000, 6.238469), epsilon = 1e-6);
|
||||
|
||||
let w = [vec![1.0], vec![1.0], vec![1.0]];
|
||||
let g = Game::ranked_with_arena(teams, &[1.0, 2.0, 0.0], &w, 0.5, &mut ScratchArena::new());
|
||||
let g = Game::ranked_with_arena(
|
||||
teams,
|
||||
&[1.0, 2.0, 0.0],
|
||||
&w,
|
||||
0.5,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
|
||||
let a = p[0][0];
|
||||
@@ -768,6 +722,7 @@ mod tests {
|
||||
&[0.0, 0.0],
|
||||
&w,
|
||||
0.25,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -795,6 +750,7 @@ mod tests {
|
||||
&[0.0, 0.0],
|
||||
&w,
|
||||
0.25,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -830,6 +786,7 @@ mod tests {
|
||||
&[0.0, 0.0, 0.0],
|
||||
&w,
|
||||
0.25,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -866,6 +823,7 @@ mod tests {
|
||||
&[0.0, 0.0, 0.0],
|
||||
&w,
|
||||
0.25,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -917,6 +875,7 @@ mod tests {
|
||||
&[1.0, 0.0, 0.0],
|
||||
&w,
|
||||
0.25,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -950,6 +909,7 @@ mod tests {
|
||||
&[1.0, 0.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -974,6 +934,7 @@ mod tests {
|
||||
&[1.0, 0.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -998,6 +959,7 @@ mod tests {
|
||||
&[1.0, 0.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -1025,6 +987,7 @@ mod tests {
|
||||
&[1.0, 0.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -1052,6 +1015,7 @@ mod tests {
|
||||
&[1.0, 0.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -1071,8 +1035,8 @@ mod tests {
|
||||
let mut t = DiffFactor::Trunc(TruncFactor::new(dt, 0.0, false));
|
||||
let mut m = DiffFactor::Margin(MarginFactor::new(dm, 5.0, 1.0));
|
||||
|
||||
let _ = t.propagate(&mut vars);
|
||||
let _ = m.propagate(&mut vars);
|
||||
let _ = t.propagate(&mut vars, 1.0);
|
||||
let _ = m.propagate(&mut vars, 1.0);
|
||||
|
||||
// Smoke: both diffs got written; their msgs are non-N_INF.
|
||||
assert!(t.msg().pi() > 0.0);
|
||||
@@ -1093,7 +1057,11 @@ mod tests {
|
||||
let weights = [vec![1.0], vec![1.0]];
|
||||
let mut arena = ScratchArena::new();
|
||||
let g = Game::scored_with_arena(
|
||||
teams, &result, &weights, 1.0, // score_sigma
|
||||
teams,
|
||||
&result,
|
||||
&weights,
|
||||
1.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut arena,
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -1112,7 +1080,8 @@ mod tests {
|
||||
vec![vec![prior], vec![prior]],
|
||||
&result,
|
||||
&weights,
|
||||
0.1, // tighter score_sigma
|
||||
0.1,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut arena2,
|
||||
);
|
||||
let p_tight = g_tight.posteriors();
|
||||
@@ -1220,6 +1189,7 @@ mod tests {
|
||||
&[1.0, 0.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -1254,6 +1224,7 @@ mod tests {
|
||||
&[1.0, 0.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -1288,6 +1259,7 @@ mod tests {
|
||||
&[1.0, 0.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -1326,6 +1298,7 @@ mod tests {
|
||||
&[1.0, 0.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let post_2vs1 = g.posteriors();
|
||||
@@ -1339,6 +1312,7 @@ mod tests {
|
||||
&[1.0, 0.0],
|
||||
&w,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
);
|
||||
let p = g.posteriors();
|
||||
@@ -1348,4 +1322,99 @@ mod tests {
|
||||
assert_ulps_eq!(p[1][0], post_2vs1[1][0], epsilon = 1e-6);
|
||||
assert_ulps_eq!(p[1][1], t_b[1].prior, epsilon = 1e-6);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn run_chain_honours_max_iter_in_convergence_options() {
|
||||
let players: Vec<R> = (0..4).map(|_| R::default()).collect();
|
||||
let teams: Vec<Vec<_>> = players.iter().map(|p| vec![*p]).collect();
|
||||
let result = vec![3.0, 2.0, 1.0, 0.0];
|
||||
let weights = vec![vec![1.0]; 4];
|
||||
|
||||
// Capped at 1 iteration: cannot fully propagate down a 4-team chain.
|
||||
let mut arena = ScratchArena::new();
|
||||
let g_capped = Game::ranked_with_arena(
|
||||
teams.clone(),
|
||||
&result,
|
||||
&weights,
|
||||
0.0,
|
||||
crate::ConvergenceOptions {
|
||||
max_iter: 1,
|
||||
..crate::ConvergenceOptions::default()
|
||||
},
|
||||
&mut arena,
|
||||
);
|
||||
let posteriors_capped = g_capped.posteriors();
|
||||
|
||||
// Same inputs, plenty of iterations: fully converged.
|
||||
let mut arena = ScratchArena::new();
|
||||
let g_full = Game::ranked_with_arena(
|
||||
teams,
|
||||
&result,
|
||||
&weights,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut arena,
|
||||
);
|
||||
let posteriors_full = g_full.posteriors();
|
||||
|
||||
// The two posteriors should differ — capped did not converge.
|
||||
let mut max_diff: f64 = 0.0;
|
||||
for (team_capped, team_full) in posteriors_capped.iter().zip(posteriors_full.iter()) {
|
||||
for (g_capped, g_full) in team_capped.iter().zip(team_full.iter()) {
|
||||
max_diff = max_diff.max((g_capped.mu() - g_full.mu()).abs());
|
||||
max_diff = max_diff.max((g_capped.sigma() - g_full.sigma()).abs());
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
max_diff > 1e-6,
|
||||
"max_iter=1 should differ from full convergence; max_diff={max_diff}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn run_chain_with_damping_converges_to_same_posterior() {
|
||||
let players: Vec<R> = (0..4).map(|_| R::default()).collect();
|
||||
let teams: Vec<Vec<_>> = players.iter().map(|p| vec![*p]).collect();
|
||||
let result = vec![3.0, 2.0, 1.0, 0.0];
|
||||
let weights = vec![vec![1.0]; 4];
|
||||
|
||||
let mut arena = ScratchArena::new();
|
||||
let g_undamped = Game::ranked_with_arena(
|
||||
teams.clone(),
|
||||
&result,
|
||||
&weights,
|
||||
0.0,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut arena,
|
||||
);
|
||||
let posteriors_undamped = g_undamped.posteriors();
|
||||
|
||||
// alpha=0.5 with extra iterations: should reach the same fixed point.
|
||||
let mut arena = ScratchArena::new();
|
||||
let g_damped = Game::ranked_with_arena(
|
||||
teams,
|
||||
&result,
|
||||
&weights,
|
||||
0.0,
|
||||
crate::ConvergenceOptions {
|
||||
alpha: 0.5,
|
||||
max_iter: 100,
|
||||
..crate::ConvergenceOptions::default()
|
||||
},
|
||||
&mut arena,
|
||||
);
|
||||
let posteriors_damped = g_damped.posteriors();
|
||||
|
||||
let mut max_diff: f64 = 0.0;
|
||||
for (team_u, team_d) in posteriors_undamped.iter().zip(posteriors_damped.iter()) {
|
||||
for (g_u, g_d) in team_u.iter().zip(team_d.iter()) {
|
||||
max_diff = max_diff.max((g_u.mu() - g_d.mu()).abs());
|
||||
max_diff = max_diff.max((g_u.sigma() - g_d.sigma()).abs());
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
max_diff < 1e-4,
|
||||
"α=0.5 should reach the same fixed point as α=1.0; max_diff={max_diff}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -96,6 +96,18 @@ impl Gaussian {
|
||||
let var = self.sigma().powi(2) + variance_delta;
|
||||
Self::from_ms(self.mu(), var.sqrt())
|
||||
}
|
||||
|
||||
/// EP damping in natural-parameter space: `α·new + (1−α)·self`.
|
||||
///
|
||||
/// Used by within-game inference to stabilise oscillating fixed-point
|
||||
/// loops on hard graphs. `alpha = 1.0` returns `new` exactly;
|
||||
/// `alpha < 1.0` shrinks each per-step update.
|
||||
pub fn damp_natural(self, new: Gaussian, alpha: f64) -> Gaussian {
|
||||
Gaussian::from_natural(
|
||||
alpha * new.pi() + (1.0 - alpha) * self.pi(),
|
||||
alpha * new.tau() + (1.0 - alpha) * self.tau(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for Gaussian {
|
||||
@@ -231,4 +243,33 @@ mod tests {
|
||||
assert!((r.pi() - expected_pi).abs() < 1e-15);
|
||||
assert!((r.tau() - expected_tau).abs() < 1e-15);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn damp_natural_alpha_one_returns_new() {
|
||||
let old = Gaussian::from_ms(1.0, 2.0);
|
||||
let new = Gaussian::from_ms(5.0, 0.5);
|
||||
let damped = old.damp_natural(new, 1.0);
|
||||
assert_eq!(damped.pi(), new.pi());
|
||||
assert_eq!(damped.tau(), new.tau());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn damp_natural_alpha_zero_returns_self() {
|
||||
let old = Gaussian::from_ms(1.0, 2.0);
|
||||
let new = Gaussian::from_ms(5.0, 0.5);
|
||||
let damped = old.damp_natural(new, 0.0);
|
||||
assert_eq!(damped.pi(), old.pi());
|
||||
assert_eq!(damped.tau(), old.tau());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn damp_natural_alpha_half_is_midpoint_in_natural_params() {
|
||||
let old = Gaussian::from_ms(1.0, 2.0);
|
||||
let new = Gaussian::from_ms(5.0, 0.5);
|
||||
let damped = old.damp_natural(new, 0.5);
|
||||
let expected_pi = 0.5 * new.pi() + 0.5 * old.pi();
|
||||
let expected_tau = 0.5 * new.tau() + 0.5 * old.tau();
|
||||
assert!((damped.pi() - expected_pi).abs() < 1e-12);
|
||||
assert!((damped.tau() - expected_tau).abs() < 1e-12);
|
||||
}
|
||||
}
|
||||
|
||||
+97
-1
@@ -594,7 +594,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
|
||||
agent.message = time_slice.forward_prior_out(&agent_idx);
|
||||
}
|
||||
} else {
|
||||
let mut time_slice = TimeSlice::new(t, self.p_draw);
|
||||
let mut time_slice = TimeSlice::new(t, self.p_draw, self.convergence);
|
||||
time_slice.add_events(composition, results, weights, kinds_chunk, &self.agents);
|
||||
|
||||
self.time_slices.insert(k, time_slice);
|
||||
@@ -838,6 +838,7 @@ mod tests {
|
||||
&[0.0, 1.0],
|
||||
&w,
|
||||
P_DRAW,
|
||||
crate::ConvergenceOptions::default(),
|
||||
&mut ScratchArena::new(),
|
||||
)
|
||||
.posteriors();
|
||||
@@ -1368,6 +1369,7 @@ mod tests {
|
||||
h.convergence = ConvergenceOptions {
|
||||
max_iter: 11,
|
||||
epsilon: EPSILON,
|
||||
alpha: 1.0,
|
||||
};
|
||||
h.converge().unwrap();
|
||||
|
||||
@@ -1685,6 +1687,7 @@ mod tests {
|
||||
.convergence(ConvergenceOptions {
|
||||
max_iter: 30,
|
||||
epsilon: 1e-6,
|
||||
alpha: 1.0,
|
||||
})
|
||||
.build();
|
||||
|
||||
@@ -1711,4 +1714,97 @@ mod tests {
|
||||
fn history_builder_rejects_zero_score_sigma() {
|
||||
let _ = History::builder().score_sigma(0.0).build();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn history_propagates_convergence_to_inner_run_chain() {
|
||||
use crate::ConvergenceOptions;
|
||||
|
||||
let events_for =
|
||||
|h: &mut History<i64, ConstantDrift, crate::observer::NullObserver, &'static str>| {
|
||||
h.event(0)
|
||||
.team(["a"])
|
||||
.team(["b"])
|
||||
.team(["c"])
|
||||
.team(["d"])
|
||||
.ranking([0u32, 1, 2, 3])
|
||||
.commit()
|
||||
.unwrap();
|
||||
};
|
||||
|
||||
let mut h_capped: History<i64, _, _, &'static str> = History::builder()
|
||||
.convergence(ConvergenceOptions {
|
||||
max_iter: 1,
|
||||
..ConvergenceOptions::default()
|
||||
})
|
||||
.build();
|
||||
events_for(&mut h_capped);
|
||||
h_capped.converge().unwrap();
|
||||
|
||||
let mut h_full: History<i64, _, _, &'static str> = History::builder().build();
|
||||
events_for(&mut h_full);
|
||||
h_full.converge().unwrap();
|
||||
|
||||
let curves_capped = h_capped.learning_curves();
|
||||
let curves_full = h_full.learning_curves();
|
||||
|
||||
let mut max_diff: f64 = 0.0;
|
||||
for (key, capped_pts) in curves_capped.iter() {
|
||||
let full_pts = curves_full.get(key).expect("agent missing in full");
|
||||
for (capped, full) in capped_pts.iter().zip(full_pts.iter()) {
|
||||
max_diff = max_diff.max((capped.1.mu() - full.1.mu()).abs());
|
||||
max_diff = max_diff.max((capped.1.sigma() - full.1.sigma()).abs());
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
max_diff > 1e-6,
|
||||
"max_iter=1 inner loop should differ from default; max_diff={max_diff}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn history_with_damping_reaches_same_fixed_point_as_undamped() {
|
||||
use crate::ConvergenceOptions;
|
||||
|
||||
let events_for =
|
||||
|h: &mut History<i64, ConstantDrift, crate::observer::NullObserver, &'static str>| {
|
||||
h.event(0)
|
||||
.team(["a"])
|
||||
.team(["b"])
|
||||
.team(["c"])
|
||||
.team(["d"])
|
||||
.ranking([0u32, 1, 2, 3])
|
||||
.commit()
|
||||
.unwrap();
|
||||
};
|
||||
|
||||
let mut h_undamped: History<i64, _, _, &'static str> = History::builder().build();
|
||||
events_for(&mut h_undamped);
|
||||
h_undamped.converge().unwrap();
|
||||
|
||||
let mut h_damped: History<i64, _, _, &'static str> = History::builder()
|
||||
.convergence(ConvergenceOptions {
|
||||
alpha: 0.5,
|
||||
max_iter: 200,
|
||||
..ConvergenceOptions::default()
|
||||
})
|
||||
.build();
|
||||
events_for(&mut h_damped);
|
||||
h_damped.converge().unwrap();
|
||||
|
||||
let curves_u = h_undamped.learning_curves();
|
||||
let curves_d = h_damped.learning_curves();
|
||||
|
||||
let mut max_diff: f64 = 0.0;
|
||||
for (key, u_pts) in curves_u.iter() {
|
||||
let d_pts = curves_d.get(key).expect("agent missing in damped");
|
||||
for (u, d) in u_pts.iter().zip(d_pts.iter()) {
|
||||
max_diff = max_diff.max((u.1.mu() - d.1.mu()).abs());
|
||||
max_diff = max_diff.max((u.1.sigma() - d.1.sigma()).abs());
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
max_diff < 1e-3,
|
||||
"α=0.5 should reach the same fixed point as α=1.0; max_diff={max_diff}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
+61
-25
@@ -133,17 +133,23 @@ impl Event {
|
||||
skills: &mut SkillStore,
|
||||
agents: &CompetitorStore<T, D>,
|
||||
p_draw: f64,
|
||||
convergence: crate::ConvergenceOptions,
|
||||
arena: &mut ScratchArena,
|
||||
) {
|
||||
let teams = self.within_priors(false, false, skills, agents);
|
||||
let result = self.outputs();
|
||||
let g = match self.kind {
|
||||
EventKind::Ranked => {
|
||||
Game::ranked_with_arena(teams, &result, &self.weights, p_draw, arena)
|
||||
}
|
||||
EventKind::Scored { score_sigma } => {
|
||||
Game::scored_with_arena(teams, &result, &self.weights, score_sigma, arena)
|
||||
Game::ranked_with_arena(teams, &result, &self.weights, p_draw, convergence, arena)
|
||||
}
|
||||
EventKind::Scored { score_sigma } => Game::scored_with_arena(
|
||||
teams,
|
||||
&result,
|
||||
&self.weights,
|
||||
score_sigma,
|
||||
convergence,
|
||||
arena,
|
||||
),
|
||||
};
|
||||
|
||||
for (t, team) in self.teams.iter_mut().enumerate() {
|
||||
@@ -165,17 +171,19 @@ pub struct TimeSlice<T: Time = i64> {
|
||||
pub(crate) skills: SkillStore,
|
||||
pub(crate) time: T,
|
||||
p_draw: f64,
|
||||
pub(crate) convergence: crate::ConvergenceOptions,
|
||||
arena: ScratchArena,
|
||||
pub(crate) color_groups: ColorGroups,
|
||||
}
|
||||
|
||||
impl<T: Time> TimeSlice<T> {
|
||||
pub fn new(time: T, p_draw: f64) -> Self {
|
||||
pub fn new(time: T, p_draw: f64, convergence: crate::ConvergenceOptions) -> Self {
|
||||
Self {
|
||||
events: Vec::new(),
|
||||
skills: SkillStore::new(),
|
||||
time,
|
||||
p_draw,
|
||||
convergence,
|
||||
arena: ScratchArena::new(),
|
||||
color_groups: ColorGroups::new(),
|
||||
}
|
||||
@@ -322,6 +330,7 @@ impl<T: Time> TimeSlice<T> {
|
||||
&result,
|
||||
&event.weights,
|
||||
self.p_draw,
|
||||
self.convergence,
|
||||
&mut self.arena,
|
||||
),
|
||||
EventKind::Scored { score_sigma } => Game::scored_with_arena(
|
||||
@@ -329,6 +338,7 @@ impl<T: Time> TimeSlice<T> {
|
||||
&result,
|
||||
&event.weights,
|
||||
score_sigma,
|
||||
self.convergence,
|
||||
&mut self.arena,
|
||||
),
|
||||
};
|
||||
@@ -382,6 +392,7 @@ impl<T: Time> TimeSlice<T> {
|
||||
}
|
||||
let range = self.color_groups.color_range(color_idx);
|
||||
let p_draw = self.p_draw;
|
||||
let convergence = self.convergence;
|
||||
|
||||
if group_len >= RAYON_THRESHOLD {
|
||||
// Obtain a raw pointer from the unique `&mut self.skills` reference.
|
||||
@@ -399,12 +410,18 @@ impl<T: Time> TimeSlice<T> {
|
||||
ARENA.with(|cell| {
|
||||
let mut arena = cell.borrow_mut();
|
||||
arena.reset();
|
||||
ev.iteration_direct(skills, agents, p_draw, &mut arena);
|
||||
ev.iteration_direct(skills, agents, p_draw, convergence, &mut arena);
|
||||
});
|
||||
});
|
||||
} else {
|
||||
for ev in &mut self.events[range] {
|
||||
ev.iteration_direct(&mut self.skills, agents, p_draw, &mut self.arena);
|
||||
ev.iteration_direct(
|
||||
&mut self.skills,
|
||||
agents,
|
||||
p_draw,
|
||||
self.convergence,
|
||||
&mut self.arena,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -426,13 +443,22 @@ impl<T: Time> TimeSlice<T> {
|
||||
// allowed within a single method body.
|
||||
let p_draw = self.p_draw;
|
||||
for ev in &mut self.events[range] {
|
||||
ev.iteration_direct(&mut self.skills, agents, p_draw, &mut self.arena);
|
||||
ev.iteration_direct(
|
||||
&mut self.skills,
|
||||
agents,
|
||||
p_draw,
|
||||
self.convergence,
|
||||
&mut self.arena,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(dead_code)]
|
||||
pub(crate) fn convergence<D: Drift<T>>(&mut self, agents: &CompetitorStore<T, D>) -> usize {
|
||||
pub(crate) fn iterate_to_convergence<D: Drift<T>>(
|
||||
&mut self,
|
||||
agents: &CompetitorStore<T, D>,
|
||||
) -> usize {
|
||||
let epsilon = 1e-6;
|
||||
let iterations = 20;
|
||||
|
||||
@@ -504,16 +530,26 @@ impl<T: Time> TimeSlice<T> {
|
||||
let teams = event.within_priors(online, forward, &self.skills, agents);
|
||||
let result = event.outputs();
|
||||
match event.kind {
|
||||
EventKind::Ranked => {
|
||||
Game::ranked_with_arena(teams, &result, &event.weights, self.p_draw, arena)
|
||||
EventKind::Ranked => Game::ranked_with_arena(
|
||||
teams,
|
||||
&result,
|
||||
&event.weights,
|
||||
self.p_draw,
|
||||
self.convergence,
|
||||
arena,
|
||||
)
|
||||
.evidence
|
||||
.ln()
|
||||
}
|
||||
EventKind::Scored { score_sigma } => {
|
||||
Game::scored_with_arena(teams, &result, &event.weights, score_sigma, arena)
|
||||
.ln(),
|
||||
EventKind::Scored { score_sigma } => Game::scored_with_arena(
|
||||
teams,
|
||||
&result,
|
||||
&event.weights,
|
||||
score_sigma,
|
||||
self.convergence,
|
||||
arena,
|
||||
)
|
||||
.evidence
|
||||
.ln()
|
||||
}
|
||||
.ln(),
|
||||
}
|
||||
};
|
||||
|
||||
@@ -621,7 +657,7 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
let mut time_slice = TimeSlice::new(0i64, 0.0);
|
||||
let mut time_slice = TimeSlice::new(0i64, 0.0, crate::ConvergenceOptions::default());
|
||||
|
||||
time_slice.add_events(
|
||||
vec![
|
||||
@@ -668,7 +704,7 @@ mod tests {
|
||||
epsilon = 1e-6
|
||||
);
|
||||
|
||||
assert_eq!(time_slice.convergence(&agents), 1);
|
||||
assert_eq!(time_slice.iterate_to_convergence(&agents), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -698,7 +734,7 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
let mut time_slice = TimeSlice::new(0i64, 0.0);
|
||||
let mut time_slice = TimeSlice::new(0i64, 0.0, crate::ConvergenceOptions::default());
|
||||
|
||||
time_slice.add_events(
|
||||
vec![
|
||||
@@ -730,7 +766,7 @@ mod tests {
|
||||
epsilon = 1e-6
|
||||
);
|
||||
|
||||
assert!(time_slice.convergence(&agents) > 1);
|
||||
assert!(time_slice.iterate_to_convergence(&agents) > 1);
|
||||
|
||||
let post = time_slice.posteriors();
|
||||
|
||||
@@ -778,7 +814,7 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
let mut time_slice = TimeSlice::new(0i64, 0.0);
|
||||
let mut time_slice = TimeSlice::new(0i64, 0.0, crate::ConvergenceOptions::default());
|
||||
|
||||
time_slice.add_events(
|
||||
vec![
|
||||
@@ -792,7 +828,7 @@ mod tests {
|
||||
&agents,
|
||||
);
|
||||
|
||||
time_slice.convergence(&agents);
|
||||
time_slice.iterate_to_convergence(&agents);
|
||||
|
||||
let post = time_slice.posteriors();
|
||||
|
||||
@@ -826,7 +862,7 @@ mod tests {
|
||||
|
||||
assert_eq!(time_slice.events.len(), 6);
|
||||
|
||||
time_slice.convergence(&agents);
|
||||
time_slice.iterate_to_convergence(&agents);
|
||||
|
||||
let post = time_slice.posteriors();
|
||||
|
||||
@@ -876,7 +912,7 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
let mut ts = TimeSlice::new(0i64, 0.0);
|
||||
let mut ts = TimeSlice::new(0i64, 0.0, crate::ConvergenceOptions::default());
|
||||
|
||||
ts.add_events(
|
||||
vec![
|
||||
|
||||
@@ -15,6 +15,7 @@ fn add_events_bulk_via_iter() {
|
||||
.convergence(ConvergenceOptions {
|
||||
max_iter: 30,
|
||||
epsilon: 1e-6,
|
||||
alpha: 1.0,
|
||||
})
|
||||
.build();
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ fn build_and_converge(seed: u64) -> Vec<(i64, trueskill_tt::Gaussian)> {
|
||||
.convergence(ConvergenceOptions {
|
||||
max_iter: 30,
|
||||
epsilon: 1e-6,
|
||||
alpha: 1.0,
|
||||
})
|
||||
.build();
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ fn record_winner_builds_history() {
|
||||
.convergence(ConvergenceOptions {
|
||||
max_iter: 30,
|
||||
epsilon: 1e-6,
|
||||
alpha: 1.0,
|
||||
})
|
||||
.build();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user