From 9e8515b7cd0f4aeee6e539e3ca86ed8428c7815e Mon Sep 17 00:00:00 2001 From: Anders Olsson Date: Tue, 4 Aug 2026 22:05:49 +0200 Subject: [PATCH] docs: refresh README and CLAUDE.md; add ingest benchmark MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CLAUDE.md architecture section still described the pre-redesign engine: its data flow named `Batch`, `Agent`, `Player` and `message.rs`, none of which have existed since T2, and the public API it listed did not match `lib.rs`. It is the first thing a fresh session reads, so it was actively misleading. Rewritten against the current module layout, with the invariants that are easy to violate — ties needing a positive `p_draw`, NaN never being convergence, log-space evidence, color contiguity, `forbid(unsafe_code)`, and ingestion-order equivalence — written down. The README Todo list had five entries that were already done, including "Time needs to be an enum": `Time` has been a trait since T2, and the `batch::compute_elapsed()` it pointed at no longer exists. The genuinely open item — cross-checking `quality()` against sublee/trueskill — stays. `benches/ingest.rs` measures one-event-per-call against a single batched call. The rest of the suite only measured batched construction, which is why the quadratic fixed earlier on this branch went unnoticed for so long. `TimeSlice::log_evidence` also hashes its target set once instead of scanning the slice per player per event, so `log_evidence_for` with many keys is no longer quadratic. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DnsaJg74eNSva3PJjK2eej --- CLAUDE.md | 106 ++++++++++++++++++++++++++++++++++------------ Cargo.toml | 4 ++ README.md | 10 ++--- benches/ingest.rs | 62 +++++++++++++++++++++++++++ src/history.rs | 6 +++ src/time_slice.rs | 7 ++- 6 files changed, 162 insertions(+), 33 deletions(-) create mode 100644 benches/ingest.rs diff --git a/CLAUDE.md b/CLAUDE.md index 66e05eb..6d2c3b5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -5,42 +5,96 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Commands ```bash -cargo build # Build the library -cargo test --lib # Run all library tests -cargo test --lib # Run a single test by name -cargo test --lib -- --nocapture # Run tests with stdout output -cargo clippy # Lint -cargo bench # Run benchmarks (criterion) +just test # Full suite across every feature combination CI checks +just check # Fast inner loop: cargo test --features approx +just lint # clippy, warnings denied +just fmt # ALWAYS nightly — rustfmt.toml uses nightly-only options +just determinism # Bit-identical posteriors at RAYON_NUM_THREADS 1/2/4/8 +just ci # Everything CI runs +cargo test --lib # A single test by name +cargo bench # Criterion benchmarks ``` -The `approx` feature enables `approx::AbsDiffEq` for `Gaussian`: -```bash -cargo test --features approx -``` +**Run tests in release too.** `debug_assert!` is compiled out there, and that +is where several defects have hidden — a debug-only run is not evidence. +`just test` includes a release job. + +### Feature flags + +- `approx` — `approx::AbsDiffEq` etc. for `Gaussian`. Most numerical goldens need it. +- `rayon` — opt-in parallel within-slice sweep and per-slice query passes. ## Architecture -This is a Rust port of [TrueSkillThroughTime.py](https://github.com/glandfried/TrueSkillThroughTime.py) — a Bayesian skill rating system that tracks skill evolution over time using Gaussian message passing. +A Rust port of [TrueSkillThroughTime.py](https://github.com/glandfried/TrueSkillThroughTime.py): +Bayesian skill rating that infers skill at every point in time, propagating +evidence both forward and backward across a history. ### Data flow ``` -History → Batch[] → Game[] → teams/players +History → TimeSlice[] → Event[] → Team[] → Item[] + ↓ + Game (factor graph) → Schedule → BuiltinFactor[] ``` -- **`History`** (`history.rs`) — top-level container. Organizes games by time into `Batch`es, runs forward/backward message passing across batches, and exposes `learning_curves()` and `log_evidence()`. -- **`Batch`** (`batch.rs`) — all games at a single time step. Runs `iteration()` to update skill estimates via `Game::posteriors()`, collecting `Skill` distributions per player. -- **`Game`** (`game.rs`) — a single match. Given teams (slices of `Gaussian`), computes posterior skill distributions using Gaussian factor graphs and `message.rs` helpers. -- **`Agent`** (`agent.rs`) — wraps a `Player` with temporal state (`last_time`, `message`). `receive()` applies time-decay (`gamma`) when the player reappears after a gap. -- **`Player`** (`player.rs`) — static configuration: prior `Gaussian`, `beta` (performance noise), `gamma` (skill drift per time unit). -- **`Gaussian`** (`gaussian.rs`) — core probability type. Stored as natural parameters (`pi = 1/sigma²`, `tau = mu/sigma²`). Arithmetic ops implement message multiplication/division in the factor graph. -- **`message.rs`** — `TeamMessage` and `DiffMessage`: intermediate factor graph messages used inside `Game`. -- **`MarginFactor`** (`factor/margin.rs`) — Gaussian observation factor on a diff variable; engaged by `Outcome::Scored`. -- **`lib.rs`** — exports the public API (`Game`, `Gaussian`, `History`, `Player`) and standalone functions (`quality()`, `pdf()`, `cdf()`, `erfc()`). Also defines global defaults: `MU=0.0`, `SIGMA=6.0`, `BETA=1.0`, `GAMMA=0.03`, `P_DRAW=0.0`, `EPSILON=1e-6`, `ITERATIONS=30`. +- **`History`** (`history.rs`) — top level. Interns keys, groups events into + `TimeSlice`s by time, runs the forward/backward sweep in `converge()`, and + answers `learning_curves()`, `current_skill()`, `log_evidence()`, + `predict_quality()`, `predict_outcome()`. Built via `HistoryBuilder`. +- **`TimeSlice`** (`time_slice.rs`) — all events at one time. Owns a + `SkillStore` and a `ScratchArena`; `iteration()` sweeps its events, using + `ColorGroups` to partition independent ones. +- **`Event`** (`time_slice.rs`) — one match. `compute()` runs inference reading + skills immutably; `apply()` folds the result back. The split is what lets a + color group run in parallel with no `unsafe`. +- **`Game`** (`game.rs`) — a single match's factor graph. `run_chain` builds the + diff chain between rank-adjacent teams and drives it to convergence. +- **`Gaussian`** (`gaussian.rs`) — natural parameters (`pi = 1/sigma²`, + `tau = mu/sigma²`). `Mul`/`Div` are the EP product/cavity: pure adds and + subtracts. Variance-space ops (`Add`, `Sub`, `exclude`, `forget`) go through + `from_mv`/`variance()` and take no square root. +- **`factor/`** — `TeamSumFactor`, `RankDiffFactor`, `TruncFactor` (ranked), + `MarginFactor` (scored), over a flat `VarStore`. `BuiltinFactor` dispatches + by enum rather than `dyn`. +- **`Schedule`** (`schedule.rs`) — drives factor propagation. `EpsilonOrMax` is + the only implementation. +- **`Competitor`** (`competitor.rs`) — per-history temporal state (`message`, + `last_time`). **`Rating`** (`rating.rs`) — static config (prior, `beta`, drift). +- **`storage/`** — `SkillStore` (per slice) and `CompetitorStore` (per history), + both dense `Vec`s indexed by `Index`. +- **`KeyTable`** (`key_table.rs`) — user key ↔ `Index`, both directions O(1). +- **`Drift`** (`drift.rs`) / **`Time`** (`time.rs`) — traits. `Time` is a *trait* + (`i64`, `Untimed`), not an enum. +- **`lib.rs`** — public exports, global defaults (`MU`, `SIGMA`, `BETA`, + `GAMMA`, `P_DRAW`, `EPSILON`, `ITERATIONS`), and the standalone `quality()`, + `cdf()`, `erfc()`. -### Key design points +### Invariants worth knowing -- `History` uses `IndexMap` (defined in `lib.rs`) to map arbitrary player keys to `Agent` state. -- Convergence is measured by the maximum `delta()` across all skill distributions; iteration stops when below `EPSILON` or after `ITERATIONS` rounds. -- The `approx` feature gates `AbsDiffEq` on `Gaussian` for use in tests — the feature is optional and only needed for approximate equality assertions. -- `time` in `History`/`Batch` is currently an `f64`; the README notes it needs to become an enum to support richer temporal states. +- **A tie needs `p_draw > 0`.** With `p_draw == 0.0` the truncation margin is + zero and the two-sided tie update evaluates `0/0`. Ingestion rejects such + events with `InferenceError::TieWithoutDrawProbability`. This includes + `Outcome::winner(w, n)` for `n >= 3`, which ties every loser. +- **NaN is never convergence.** Comparisons against NaN are all false, so + `tuple_gt` reads NaN as "below epsilon". Use `step_converged` / + `step_is_finite`, never `!tuple_gt(..)` alone. +- **Evidence accumulates in log space.** A linear product over a long diff + chain underflows to zero, and `ln(0)` is `-inf`. +- **Colors are contiguous.** `recompute_color_groups` reorders events so each + color occupies one range; `ColorGroups::groups_are_contiguous` asserts it. +- **The crate is `#![forbid(unsafe_code)]`.** Keep it that way. +- **Ingestion order must not change the answer.** Events added one at a time + must converge to the same fixed point as the same events batched — see + `tests/ingestion_equivalence.rs`. + +### Testing notes + +- Numerical goldens are cross-validated against the Python/Julia reference. + Some are *convergence residuals*, not exact values; treat a small movement + as suspicious but check whether the new value is closer to the analytic + truth (symmetric fixtures converge to their prior mean exactly) before + assuming a regression. +- `tests/degenerate_inputs.rs` covers empty/boundary/error paths, + `tests/ingestion_equivalence.rs` covers batching order, `tests/quality.rs` + covers N-group quality, `tests/determinism.rs` covers thread counts. diff --git a/Cargo.toml b/Cargo.toml index 6e74888..2774717 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,10 @@ harness = false name = "scored" harness = false +[[bench]] +name = "ingest" +harness = false + [dependencies] approx = { version = "0.5.1", optional = true } rayon = { version = "1", optional = true } diff --git a/README.md b/README.md index cf009c1..cf00ad6 100644 --- a/README.md +++ b/README.md @@ -96,8 +96,8 @@ h.converge().unwrap(); - [x] Implement approx for Gaussian - [x] Add more tests from `TrueSkillThroughTime.jl` -- [ ] Add tests for `quality()` (Use [sublee/trueskill](https://github.com/sublee/trueskill/tree/master) as reference) -- [ ] Benchmark Batch::iteration() -- [ ] Time needs to be an enum so we can have multiple states (see `batch::compute_elapsed()`) -- [ ] Add examples (use same TrueSkillThroughTime.(py|jl)) -- [ ] Add Observer (see [argmin](https://docs.rs/argmin/latest/argmin/core/trait.Observe.html) for inspiration) +- [x] Generalise a time axis — `Time` is now a trait (`Untimed`, `i64`), not an enum +- [x] Add examples (`examples/atp.rs`, `examples/scored.rs`) +- [x] Add Observer (`Observer` / `NullObserver`) +- [x] Benchmark the inference loop (`benches/batch.rs`, `benches/history_converge.rs`, `benches/ingest.rs`) +- [ ] Cross-check `quality()` against [sublee/trueskill](https://github.com/sublee/trueskill/tree/master) — N-group support works and is covered by invariants, but no reference values are asserted diff --git a/benches/ingest.rs b/benches/ingest.rs new file mode 100644 index 0000000..0bac7a3 --- /dev/null +++ b/benches/ingest.rs @@ -0,0 +1,62 @@ +//! Ingestion cost: one event per call versus one batched call. +//! +//! The rest of the suite only measures batched construction, which is why a +//! quadratic in the incremental path went unnoticed — `record_winner` and +//! `event(..).commit()` each ingest a single event, so a caller looping over a +//! match feed takes that path. + +use std::hint::black_box; + +use criterion::{BenchmarkId, Criterion, criterion_group, criterion_main}; +use smallvec::smallvec; +use trueskill_tt::{Event, History, Member, Outcome, Team}; + +fn events(n: usize, time: i64) -> Vec> { + (0..n) + .map(|i| Event { + time, + teams: smallvec![ + Team::with_members([Member::new(format!("p{}", 2 * i))]), + Team::with_members([Member::new(format!("p{}", 2 * i + 1))]), + ], + outcome: Outcome::winner(0, 2), + }) + .collect() +} + +fn bench_ingest(c: &mut Criterion) { + let mut group = c.benchmark_group("ingest"); + + for n in [250usize, 500, 1000] { + group.bench_with_input(BenchmarkId::new("one-at-a-time", n), &n, |b, &n| { + b.iter_batched( + || events(n, 0), + |evs| { + let mut h: History = History::builder_with_key().build(); + for ev in evs { + h.add_events(std::iter::once(ev)).unwrap(); + } + black_box(h.time_slices_len()) + }, + criterion::BatchSize::SmallInput, + ); + }); + + group.bench_with_input(BenchmarkId::new("single-batch", n), &n, |b, &n| { + b.iter_batched( + || events(n, 0), + |evs| { + let mut h: History = History::builder_with_key().build(); + h.add_events(evs).unwrap(); + black_box(h.time_slices_len()) + }, + criterion::BatchSize::SmallInput, + ); + }); + } + + group.finish(); +} + +criterion_group!(benches, bench_ingest); +criterion_main!(benches); diff --git a/src/history.rs b/src/history.rs index 3cfec49..b83efbd 100644 --- a/src/history.rs +++ b/src/history.rs @@ -312,6 +312,12 @@ impl, O: Observer, K: Eq + Hash + Clone> History usize { + self.time_slices.len() + } + /// Learning curves for all competitors, keyed by their user-facing key. /// /// Note: `key(idx)` is O(n) per lookup; this method is therefore O(n²) diff --git a/src/time_slice.rs b/src/time_slice.rs index 8c95927..9b9687b 100644 --- a/src/time_slice.rs +++ b/src/time_slice.rs @@ -587,6 +587,9 @@ impl TimeSlice { forward: bool, agents: &CompetitorStore, ) -> f64 { + // Hashed once rather than scanned per player per event, so a + // `log_evidence_for` with many keys is not quadratic. + let target_set: std::collections::HashSet = targets.iter().copied().collect(); // log_evidence is infrequent; a local arena avoids needing &mut self. let mut arena = ScratchArena::new(); @@ -636,7 +639,7 @@ impl TimeSlice { .teams .iter() .flat_map(|team| &team.items) - .any(|item| targets.contains(&item.agent)) + .any(|item| target_set.contains(&item.agent)) }) .map(|event| run_event(event, &mut arena)) .sum() @@ -648,7 +651,7 @@ impl TimeSlice { .teams .iter() .flat_map(|team| &team.items) - .any(|item| targets.contains(&item.agent)) + .any(|item| target_set.contains(&item.agent)) }) .map(|event| event.log_evidence) .sum()