The last two items on #23.
`OwnedGame::new` and `new_scored` cloned the whole team structure to hand one
copy to `Game` and keep another. But `Game` takes the teams by value and is
dropped at the end of the constructor, so the vec can simply be taken back out
of it — the clone existed only because nobody looked at the lifetime.
`add_events_with_prior` deep-cloned each event's composition, results and
weights when chunking events into per-timestamp groups. Nothing reads those
three after the chunking loop (the agent-collection pass and the tie pre-check
both run before it), so the elements are now moved out with `mem::take`.
That soundness argument rests entirely on `o` being a permutation: visiting an
index twice would take an already-emptied vec and silently produce an event
with no teams rather than failing. Since that would be invisible, there is now
a debug_assert checking the permutation property directly, next to the comment
explaining why the code depends on it.
Verified on 1.85.0 as well as the local toolchain — an MSRV break in this
change would otherwise only surface in CI.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
Closes the last open item in #25. `cargo clippy -W missing_errors_doc
-W missing_panics_doc -W must_use_candidate -W doc_markdown` went from 56
warnings to zero.
The 13 hand-written sections name the actual variants each function returns
rather than gesturing at "an error". Establishing that meant reading the error
paths — `Game::ranked` alone returns four distinct variants, and `record_draw`
can hit TieWithoutDrawProbability where `record_winner` provably cannot, since
a two-team decisive outcome has nothing to tie. Documenting those as
interchangeable would have been worse than leaving them undocumented, because
a reader would trust it.
Two existing doc comments already described panics in prose but not under a
`# Panics` heading, so neither rustdoc nor clippy surfaced them:
`Outcome::winner` and `EventBuilder::weights`. Both now carry the heading, and
`Outcome::winner` gained the note that it ties every loser, so `n >= 3` needs a
positive p_draw — the crate's easiest error to hit by accident.
The 43 mechanical fixes (31 `#[must_use]` on pure accessors, 11 missing
backticks) were applied with `cargo clippy --fix`. `#[must_use]` on Gaussian's
arithmetic and on `posteriors()` matters: discarding those results is always a
bug, and until now nothing said so.
Also documented why `[profile.release] debug = true` exists — cargo-flamegraph
needs the symbols, and library profile settings are ignored downstream, so it
reads as an oversight without the note.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
There was no CI of any kind — no workflows directory at all — despite a full
release pipeline (release.toml, cliff.toml, a maintained CHANGELOG, three
tagged releases). The workflow covers the feature combinations that actually
have distinct behaviour, including a release-profile job: `debug_assert!` is
compiled out there, which is exactly where the validation this branch added
has to hold, and a debug-only suite would never have seen it. Determinism is
checked at RAYON_NUM_THREADS of 1, 2, 4 and 8.
The Justfile gains test/lint/fmt/determinism recipes so the same checks run
locally with one command, and `just ci` runs the lot.
`Cargo.toml` had only name, version and edition, so `cargo publish` would
have been rejected. Added description, repository, readme, keywords,
categories, exclude, and `rust-version = "1.85"` — the edition-2024 floor,
now verified by a CI job. Two let-chains introduced earlier on this branch
would have pushed that to 1.88; they are rewritten to keep the floor where
it was.
`src/lib.rs` had no `//!` header at all, so the docs.rs landing page would
have been a bare symbol list — conspicuous given every other module has one.
It now explains what Through Time does differently, and carries three
runnable examples (which `cargo test --doc` checks, where previously there
was nothing to check), including the draw/p_draw interaction that is the
easiest way to get an error out of this crate.
`cargo publish --dry-run` now packages and verifies cleanly. The only
remaining blocker is `license`, which is yours to choose — noted as a TODO
in the manifest rather than picked unilaterally.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnsaJg74eNSva3PJjK2eej
Convergence configuration had two disagreeing sources of truth and one
misleading report:
- `EpsilonOrMax::default()` capped at 10 iterations while
`ConvergenceOptions::default()` allowed 30, and which applied depended on
whether inference went through `run_chain` or a `Schedule`. The schedule
default now derives from `ConvergenceOptions`.
- A graph with no iterating factors reported `converged: false` with an
infinite step, despite being at its fixed point after the setup pass. It
now reports converged with a zero step.
- `TimeSlice::iterate_to_convergence` hard-coded an epsilon and a
20-iteration cap matching neither. It reads `self.convergence` and is
scoped to `#[cfg(test)]`, which is all it was ever used by.
`HistoryBuilder::p_draw` and `::convergence` now validate their arguments
like `score_sigma` already did, instead of accepting a negative `p_draw` or
an `alpha` of zero — the latter leaves every EP update unapplied, so
inference silently returns the priors.
Removing the `#[allow(dead_code)]` masks let the compiler report what they
were hiding: four `OwnedGame` fields that were stored and never read, two
`ColorGroups` helpers and three `SkillStore` helpers used only by tests, and
`iterate_to_convergence` above. Test-only items are now `#[cfg(test)]` and
the unread fields are gone.
Also exported `HistoryBuilder`, which was public but unreachable — callers
could chain `History::builder()` but could not name the type — and added
`Rating::{prior, beta, drift}` and `Index::get`, so handles the API hands
out can be read back.
Two goldens moved, both convergence residuals rather than exact values:
`iterate_to_convergence` now runs to 30 iterations instead of 20, landing
nearer the symmetric truth of 25.0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnsaJg74eNSva3PJjK2eej
`Add`, `Sub`, `exclude` and `forget` combined variances by way of standard
deviations: `sigma()` takes a square root, `.powi(2)` squares it away,
`var.sqrt()` takes another, and `from_ms` squares that one back. Three roots
to compute a value that is `1/pi` all along.
They now go through `variance()` and a new `from_mv(mu, var)`, which skip
both conversions. `Sub` is the hot one — `RankDiffFactor::propagate` is
`a - b`, run for every adjacent team pair on every forward and backward
sweep of every EP iteration.
`run_chain` also stopped recomputing each team's weighted performance in the
likelihood loop; the fold is already in `arena.team_prior`, indexed by the
sorted position the loop has in hand. Each `performance()` is itself a
`forget`, so the duplicate cost scaled with players per team.
Measured on this machine, before and after, same fixtures:
Batch::iteration 23.57us -> 19.31us (-18%)
scored_history_60_events 1.071ms -> 983us (-8%)
The `Gaussian::add`/`sub` microbenchmarks cannot resolve the change: they
sit at ~234ps against a ~218ps floor that `mul`/`div` also hit, so the
harness overhead dominates a single operation.
One golden moved. Two identical competitors drawing must land on their
shared prior mean exactly, by symmetry; the root-free path now returns
25.0 where the reference transcription recorded 24.999999 — that value
rounded to six decimals. Asserting a six-decimal transcription at
epsilon 1e-6 left no headroom, so the expectation is now the exact value.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnsaJg74eNSva3PJjK2eej
Per-link evidence was multiplied in linear space and logged only at the
end. Each link contributes a probability in (0, 1], so the product over an
n-team game decays geometrically: around a thousand links it flushes to
exactly 0.0 and `ln(0.0)` is `-inf`, which then propagates through the sum
in `History::log_evidence_internal` and takes the whole history with it.
`Game::free_for_all` builds one team per player, so this is reachable at
the competitor counts the T3 benchmarks target.
`Game`, `OwnedGame`, and `time_slice::Event` now carry `log_evidence`
directly, summed over links rather than multiplied then logged.
The cached per-link evidence is also floored at `f64::MIN_POSITIVE`. It
could legitimately reach zero or go negative: `1.0 - cdf(..)` rounds to
zero for a near-certain outcome, and the `erfc` approximation carries
~1e-7 error so `cdf` can exceed 1.0 and make the difference negative —
`ln` of which is NaN.
Existing log-evidence goldens are unchanged, confirming the accumulation
is numerically equivalent in the range where the old form worked.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnsaJg74eNSva3PJjK2eej
A tie with `p_draw == 0.0` produced NaN posteriors in release builds and
`converge()` reported `converged: true`, because every comparison against
NaN is false and `tuple_gt` therefore read NaN as "below epsilon".
Two independent defects, fixed together:
- Ingestion now rejects tied outcomes when the draw probability is zero,
promoting the existing `debug_assert!` in `Game::ranked_with_arena` to a
real `InferenceError::TieWithoutDrawProbability`. Validation sits in
`add_events_with_prior`, the chokepoint every route reaches — including
`record_draw`, which bypasses `Outcome` entirely.
- `converge()` treats a non-finite step as failure and returns
`InferenceError::NonFiniteResult` rather than claiming convergence.
Also in this change:
- `History::converge()` on an empty history returned a `usize` underflow
panic from `0..len()-1`; it now short-circuits to a zero-iteration report.
- `Outcome::scores_with_sigma` no longer panics on a non-positive sigma;
the value is validated at ingestion so callers get an error instead.
- `InferenceError` gains `WrongOutcomeKind`, replacing the misuse of
`MismatchedShape` for variant mismatches (which rendered as the nonsense
"expected length 0, got 0"), and is now `#[non_exhaustive]`.
Note `Outcome::winner(w, n)` for n >= 3 ties every loser, so those events
now require a positive `p_draw`. They previously returned NaN.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnsaJg74eNSva3PJjK2eej
Two end-to-end tests on a 4-team ranked game:
- max_iter=1 produces measurably different posteriors than the default,
proving run_chain reads convergence.max_iter
- alpha=0.5 with extra iterations reaches the same fixed point as
alpha=1.0, proving damping doesn't break convergence on benign graphs
Game and OwnedGame gain a convergence: ConvergenceOptions field set at
construction. Game::{ranked,scored} forward options.convergence into
OwnedGame::{new,new_scored} (previously dropped on the floor).
{ranked,scored}_with_arena take it as a parameter. run_chain reads
self.convergence.{epsilon, max_iter, alpha} instead of hardcoded
1e-6 / 10 / undamped. DiffFactor::propagate gains an alpha parameter
and dispatches into Trunc/MarginFactor::propagate_with_alpha.
In-tree callsites in src/time_slice.rs and src/history.rs pass
ConvergenceOptions::default(). Pre-existing T2 fallout in tests,
benches, and the atp example (struct literals missing the new alpha
field) is fixed by adding alpha: 1.0 so the workspace builds clean.
Default alpha is 1.0, so all 96 lib + 27 integration test goldens
remain bit-equal.
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.
logaritmiskandClaude Opus 5