The note claimed `cargo publish` rejects a crate without `license`. That is
true only for crates.io; publishing to an alternative registry does not check
it, verified by a dry run against kellnr that packages and verifies cleanly.
Staying unlicensed is a deliberate choice, so the note now says that and states
the actual consequence — all-rights-reserved by default — rather than a
mechanical blocker that does not exist.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
`cargo publish --dry-run` packaged 48.1 MiB (8.6 MiB compressed) for a library
whose source is 312 KB. All of it was examples/atp.csv, a tennis dataset the
atp example reads.
examples/atp.rs opens it by relative path at runtime rather than include_str!,
so excluding the data still compiles and `cargo package --verify` still builds
every target — the example just needs the file fetched from the repo to run.
Packaged size is now 360.8 KiB / 83.1 KiB compressed, a 133x reduction. Every
consumer would otherwise have paid that download.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
Mirrors the textus setup, adapted for a single crate rather than a workspace.
Cargo.toml gains publish = ["kellnr"], which does double duty: it points
cargo-release at the private registry and makes an accidental `cargo publish`
to crates.io a hard error rather than an irreversible mistake.
.cargo/config.toml is committed rather than left to a per-user
~/.cargo/config.toml. Without it a fresh clone, a new machine, or CI fails
with "registry index was not found in any configuration: kellnr" before
compiling anything. The index URL is not a secret; the token stays in
~/.cargo/credentials.toml, or CARGO_REGISTRIES_KELLNR_TOKEN in CI.
release.toml flips publish from false to true and pins push = false, so the
Justfile recipe pushes last — after tags and publish have both succeeded.
The git-cliff pre-release hook is unchanged.
cliff.toml gained a Breaking Changes group. Its commit_parsers matched on type
alone with conventional_commits = false, so `refactor!: remove the inert online
flag` rendered as an ordinary Refactor bullet and the break was invisible in
the generated changelog. The new parsers match a `!` subject and a
BREAKING CHANGE body, and must precede the type parsers because the first match
wins. The unreleased section now opens with the break, which matters because
the next release is the one that removes HistoryBuilder::online.
The release recipe runs `just ci` before cutting: cargo-release only
verify-compiles the packaged crate and publishing cannot be undone, and the
release profile is where this crate's defects have historically hidden.
Still unpublishable: Cargo.toml has no `license`. That is a deliberate TODO,
not an oversight.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
#29 — log_evidence and log_evidence_for took &mut self while mutating
nothing. Loosening them to &self is not source-breaking for ordinary callers
(a &mut reborrows as & transparently) and brings them in line with the
filtered_* accessors added last week.
Not the mechanical change it looked like: under the rayon feature the closure
in log_evidence_internal captured all of &self rather than just the competitor
store, which drags KeyTable<K> in and demands K: Sync from every caller. That
compiled while the method took &mut self and stopped compiling the moment it
did not. Binding `let agents = &self.agents;` before the closure narrows the
capture; the comment there says why, because the next person to inline it will
reintroduce the bound.
#31 — TimeSlice::add_events constructed Skill with ..Default::default() while
filtered_step spells every field out. The design relies on a new Skill field
being a compile error at construction sites rather than a silent default, and
that tripwire only fired at one of the two. Now both.
#28 — log_evidence_internal's `forward` flag is a genuine forward-only
quantity only on a history that has never been converged, because iteration
alternates sweeps and the likelihood feeding the forward message absorbs
backward information from the second iteration onward. Documented, with a
pointer to filtered_log_evidence for the quantity that survives convergence.
That trap is one function away from the one #19 was about.
#23 — color_greedy carried #[allow(dead_code)] despite being called by
recompute_color_groups: a mute button on a live function, which is the
specific complaint in that issue.
#27 was already fixed — the guard landed in f4e2922 and the issue was filed
against 7742b2b, which merge-base confirms predates it — but nothing pinned
it. Added the issue's own reproduction, which matters because the two profiles
fail differently and a debug-only test would miss the release path. Removing
both guards reproduces the issue verbatim: "attempt to subtract with overflow"
in debug, "index out of bounds: the len is 0 but the index is
18446744073709551615" in release.
Also amended the filtered-estimates spec (#30): the tolerance-not-bit-identity
caveat is conservative. Forcing the scratch onto the sequential sweep instead
of the grouped one — a far larger perturbation than a permuted event order —
still agrees within 1e-8 under tight convergence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
filtered_learning_curve's signature mirrors learning_curve, which is cheap
per key — so the mirroring trained callers to assume this one is too. It is
a full forward pass per call, making the natural loop over competitors
O(competitors * events). The doc now says so in complexity terms and points
multi-key callers at the plural form.
filtered_log_evidence claimed each event is scored "using only what was
known before it". That is exact for a slice holding one event, but events
sharing a timestamp inform each other through the within-slice sweep, so
the honest claim is "before that time". The behaviour is deliberate and
matches log_evidence's own convention; only the promise was too strong.
This branch exists because a feature's documentation was quietly false.
Shipping it with two more overstated doc comments would be a poor joke.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
The bracket test proves the feature works on one fixture. These pin the bug
class:
- Invariance to converge(). This is the one that matters. Reading
skill.forward instead of the carried message makes it fail immediately,
because converge() alternates sweeps and contaminates skill.forward with
backward information from the second iteration onward. That is the
property a stored field cannot have, and the reason issue #19's proposed
fix would not have worked.
- Invariance to ingestion order, the crate's standing invariant.
- One slice has no future to propagate back, so filtered equals smoothed.
- Empty history yields zero and empty maps.
Agreement is to 1e-8 under tight convergence rather than bit-identity:
iteration recomputes the colour partition only when from == 0, so an
incrementally built slice keeps insertion order until the first converge()
reorders it, and the scratch clone inherits whichever order it finds. Same
fixed point, different path to it.
learning_curve returns post-convergence posteriors, so every point is
smoothed: the estimate at a given date incorporates rounds played years
later. On ustat's data that starts six players' curves already spread apart
at sigma 0.9-1.6 against a prior of 6.0, barely moving thereafter.
filtered_learning_curve plots the same competitor on forward-only
information, so everyone starts at the prior and fans out. It could not be
reconstructed from the public API before: a caller could only refit over
events[0..k] for every k, which is O(n^2) fits for something one forward
pass already computes.
Scores every event on what was known before it, rather than on priors that
carry information from events which had not happened yet. This is the
quantity HistoryBuilder::online promised and never delivered.
The pass walks slices in time order carrying its own forward messages, and
per slice runs the unmodified production sweep on a scratch copy whose
backward message is left improper. Reusing iterate_to_convergence rather
than reimplementing inference means a competitor playing twice at one time
is handled by the same within-slice EP that converge() uses, instead of
being approximated the way the old evidence paths approximated it.
Nothing is stored on Skill and nothing on self is mutated, so the result is
independent of whether converge() has run — the property a stored field
cannot have.
Skill.online was initialised to N_INF and assigned nowhere, so
HistoryBuilder::online(true) made every rating improper and log_evidence()
reported n * ln(0.5) — every game scored as a coin flip. The value is finite
and plausible, which is why it went unnoticed.
The default was false, so no existing result changes. A working replacement
lands next; a stored field cannot hold the quantity, because converge()
alternates sweeps and contaminates skill.forward with backward information
from the second iteration onward.
Also renames a test binding from ..._online to ..._forward: it passes the
forward flag, and the two senses being conflated is how this survived.
Five tasks: delete the inert online machinery, add filtered_log_evidence,
add the two learning-curve methods, pin the invariants, record the API break.
Two spec corrections fell out of writing it. The spec claimed filtered results
would be bit-identical before and after converge(); they cannot be. iteration
recomputes the colour partition only when from == 0, so a slice built by
repeated appends keeps insertion order until the first converge() reorders it,
and the scratch clone inherits whichever order it finds — same fixed point,
different path. Corrected to agreement within 1e-8 under tight convergence,
matching the house pattern in tests/ingestion_equivalence.rs. The spec also
declared filtered_pass as Vec<(T, Vec<(Index, Gaussian)>)>, which cannot carry
the evidence its own step 3 harvests; it returns Vec<(T, FilteredStep)>.
CHANGELOG.md is generated by git-cliff, so the spec's "CHANGELOG records the
API break" cannot be satisfied by editing the file — it regenerates. Task 5
records the break through the commit subject and verifies the generated output
instead. cliff.toml has no breaking-change parser at all, which the task is
told to report rather than work around.
An adversarial reviewer checked the plan against the source before this commit
and found four real defects, all in plan text, none in the design:
- Two prescribed mutations provably could not fail their named tests. The
learning-curve mutation altered only what filtered_pass writes after a slice,
while the test inspected filtered[0], which is computed from an empty message
map. Fixed by asserting monotonic mu across the whole curve.
- The ingestion-order fixture used four distinct timestamps, giving one event
per slice — the exact degenerate shape ingestion_equivalence.rs documents as
the weak case, making the assertion true by construction. Fixed to several
events per timestamp with shared competitors.
- filtered_learning_curves was never asserted for content, only for emptiness
on an empty history.
- A doc comment restated learning_curves' claim that key(idx) is O(n) and the
method O(n^2). KeyTable::key is self.reverse.get(idx.0) — O(1) — and the
type's own doc says so. The claim predates reverse becoming a Vec. The plan
now corrects the original at history.rs:323 rather than copying it.
The reviewer confirmed the central claim by tracing the call graph: N_INF is
{pi: 0, tau: 0} and Mul is a natural-parameter add, so it is an exact
multiplicative identity, and the only write to skill.backward in the crate is
in new_backward_info, reachable only from History::iteration and never from
iterate_to_convergence under either rayon cfg.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
`HistoryBuilder::online(true)` is inert: it flips a flag that reaches
`Item::within_prior`, which reads `Skill.online` — a field initialised to
`N_INF` and assigned nowhere. So `log_evidence()` under that setting reports
`n * ln(0.5)`, every game scored as a coin flip. The number is finite and
plausible, which is why nothing caught it.
Issue #19 proposed populating the field during the forward pass. That does
not work, and the reason shapes the whole design. `new_forward_info` sets
`skill.forward` from the previous slice's `forward_prior_out`, which is
`skill.forward * skill.likelihood`; `History::iteration` alternates backward
and forward sweeps, so from the second iteration onward that likelihood has
already absorbed backward information. After `converge()`, `skill.forward`
is a smoothed quantity — and so is anything written from it.
The same reasoning condemns the neighbouring `forward: bool` flag, which is
a filtering quantity only on a history that was never converged. That is why
the test at history.rs:1183 can assert the two evidences are equal. Left
alone here; recorded as a follow-up.
The design is a read-only forward-only pass instead: walk slices in time
order carrying their own forward messages, and per slice build a scratch
clone whose `backward` is `N_INF`, then run the unmodified production sweep
on it. Reusing `iterate_to_convergence` rather than reimplementing inference
means a competitor playing twice at one time is handled by the same
within-slice EP that `converge()` uses, instead of being approximated the
way today's evidence paths approximate it. Nothing is stored on `Skill`,
which drops 16 bytes and helps #17 regardless.
Three methods ship — `filtered_log_evidence`, `filtered_learning_curves`,
`filtered_learning_curve` — all taking `&self`. The second consumer is
ustat, whose learning curves start already collapsed to sigma 0.9-1.6
against a prior of 6.0 because every point is smoothed; the filtered view
cannot be reconstructed from the public API today except by O(n^2) refits.
The red test brackets the issue's own fixture strictly between 5*ln(0.5) and
the batch evidence, so neither "still inert" nor "accidentally smoothed"
passes. The invariant that would have caught this bug class is that filtered
results are identical before and after `converge()` — exactly what a stored
field cannot give.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnsaJg74eNSva3PJjK2eej
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
The parallel color-group sweep passed a `*mut SkillStore` through a `usize`
and cast it back inside the rayon closure, so every worker materialised its
own `&mut SkillStore` to the same store. Two live `&mut` to one object is an
aliasing violation whatever the workers subsequently touch — `&mut` carries
`noalias` down to LLVM — and laundering the pointer through `usize` also
discarded provenance. The existing SAFETY comment argued element
disjointness, which is true and is why nothing miscompiled in practice, but
it is not the property the aliasing rules ask about.
Events in a color group touch disjoint agents, so none can observe another's
writes. That makes the sweep separable rather than merely safe-in-practice:
`Event::compute` runs inference over shared `&self.skills` with no mutation,
and `Event::apply` folds the results in afterwards in index order. No
`unsafe`, no aliasing argument, and the apply order does not depend on which
worker finished first, so results stay bit-identical across thread counts.
The crate now contains no `unsafe` at all, locked in with
`#![forbid(unsafe_code)]`.
Splitting compute from apply also removes the duplicated sweep body: the
`from > 0` branch of `TimeSlice::iteration` was a verbatim copy of
`iteration_direct`, and both now share one implementation.
Cost, measured on the three `history_converge` workloads (sequential vs
parallel, this machine):
500x100@10perslice 4.02ms -> 4.21ms
2000x200@20perslice 19.70ms -> 19.76ms
1v1-5000x50000 11.75ms -> 10.46ms
The deferred apply gives back part of the parallel win on the only workload
where rayon ever helped (1.12x here, against the 1.3x T3 reported), and the
sequential path is unchanged. Trading a fraction of a 1.3x speedup on one
pathological shape for the removal of undefined behaviour is the right side
of that bargain.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnsaJg74eNSva3PJjK2eej
`add_events_with_prior` advanced `k` past the slice it had written when it
created a new one, but not when it appended to an existing one. The trailing
forward-refresh loop therefore started *on* the slice just modified and ran
`new_forward_info` over it again.
That is not merely redundant work. The loop immediately above it sets each
agent's message to `forward * likelihood` for that slice, and
`new_forward_info` then assigns `skill.forward = message.forget(drift)` —
folding the slice's own likelihood back into its own forward prior. The
skills it produced depended on how events had been batched.
Ingesting one event at a time now converges to the same fixed point as
ingesting the same events in a single call, which it previously did not:
for five events sharing a timestamp, competitor `a` converged to
mu=7.44 sigma=3.90 batched versus mu=7.99 sigma=3.10 incrementally. Both
runs had converged; the gap was not a convergence residual.
The numerical goldens never caught this because they all ingest in one call
with a distinct timestamp per event, so the append-to-existing-slice branch
is never taken. `tests/ingestion_equivalence.rs` covers it directly, and
asserts convergence before comparing so that a residual cannot be mistaken
for agreement.
Removing the redundant re-inference also removes the dominant cost of
incremental ingestion, which was quadratic in the number of events already
in the slice:
events before after speedup
500 45.8ms 1.1ms 42x
1000 179.5ms 2.8ms 64x
2000 721.8ms 9.9ms 73x
4000 2.9s 35.4ms 82x
Ingesting one at a time is now 1.8x a single batched call, down from 148x.
Two supporting changes are included:
- Color groups are rebuilt lazily rather than on every append. Nothing
reads the partition between an append and the next full sweep, so the
per-append rebuild was pure waste.
- `ColorGroups::groups_are_contiguous` is asserted after each rebuild and
in `color_range`. The parallel sweep derives one `&mut` sub-slice per
color from those ranges and relies on them being disjoint; that invariant
was established by construction but never checked.
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
`quality()` was two-group-only in three separate ways, and
`History::predict_quality` inherited all of them.
- The contrast-matrix column counter tracked two positions with two
variables that only agree on the first row, so three or more groups wrote
past the end of the row and panicked with an out-of-bounds index. The
negative block always begins immediately after the positive one, so the
second counter is unnecessary.
- `Matrix::inverse` was implemented only for the 1x1 case and otherwise
`panic!("eh, okey")`. It now uses LU decomposition with partial pivoting,
which also replaces the recursive cofactor `determinant` — that was O(n!)
and allocated a `Vec` per minor, so a 10-team match needed 362,880 terms.
- Degenerate inputs (zero groups, one group, empty groups) underflowed or
produced NaN. They now assert with a message naming the requirement.
`Matrix` also gains dimension checks on multiply/add and bounds checks on
indexing, and loses the now-unused `adjugate`/`minor` cofactor path.
The two-group golden is unchanged. N-group behaviour is covered by
invariants — permutation invariance, and quality falling as a skill gap
widens — since no reference values were available to compare against; the
sublee/trueskill cross-check remains open.
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
EP message cancellation can leave a Gaussian's precision (pi) a tiny
negative value — round-off of exactly zero. mu()/sigma() only special-cased
pi == 0, so sigma() computed 1/sqrt(pi) = NaN for pi < 0. That NaN flowed
through the moment-space Sub in the game diff-chain and poisoned every skill
in the slice once it grew past ~75 competitors, making converge() return
all-NaN on real-scale histories (regression vs 0.1.0, which stored sigma
directly). Guard pi <= 0.0 in both accessors (improper Gaussian: mu 0,
sigma infinite), matching the existing pi == 0 handling.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three integration tests on a 2-team scored event:
- inheritance: Outcome::scores(...) with no override produces
bit-equal posteriors to the same outcome wrapped in
scores_with_sigma(scores, history.score_sigma)
- override-supersedes-default: scores_with_sigma(scores, X) with
history score_sigma(Y) produces bit-equal posteriors to
scores(...) with history score_sigma(X), AND differs measurably
from scores(...) with history score_sigma(Y)
- builder threading: EventBuilder::scores_with_sigma reaches the
ingest path identically to the Outcome constructor
Adds EventBuilder::scores_with_sigma, the fluent-builder ergonomic
mirror of Outcome::scores_with_sigma. Lets users write
h.event(t).team(...).team(...).scores_with_sigma([..], sigma).commit()
to set a per-event score_sigma override.
Outcome::Scored shape changes from tuple to struct:
{ scores, sigma: Option<f64> }. New constructor scores_with_sigma
sets sigma=Some(s) and debug-asserts s > 0.0; existing scores(I)
constructor keeps its signature and builds with sigma=None internally.
team_count, as_scores, as_ranks accessor pattern matches updated.
History::add_events resolves sigma.unwrap_or(self.score_sigma) at the
ingest arm, so downstream EventKind::Scored stays a plain f64 and
TimeSlice / run_chain need zero changes.
Breaking change to the public Outcome::Scored variant shape
(acceptable in 0.1.x). Bit-equal for callers using the no-override
path because the resolution falls through to self.score_sigma exactly
as before.
Outcome::Scored becomes a struct variant with an Option<f64> sigma
field. None inherits HistoryBuilder::score_sigma; Some(s) overrides
per event. Resolved at ingest time so EventKind::Scored stays a plain
f64 and TimeSlice/run_chain need zero changes. New constructors
Outcome::scores_with_sigma and EventBuilder::scores_with_sigma cover
the override path; existing scores(..) keeps its signature with
sigma=None internally.
Breaking change to Outcome::Scored variant shape (tuple → struct);
acceptable in 0.1.x. Closes the last item from the T4-MarginFactor
deferred wishlist.
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.
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.
Removes the temporary #[allow(dead_code)] on TimeSlice::convergence
that was added in the prior commit.
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.
Also updated benches/batch.rs which has a fourth TimeSlice::new
callsite (not enumerated in the plan -- only src/ files were).
Closes the gap between HistoryBuilder::convergence(opts) and the
within-game inference loop. TimeSlice gains a convergence field;
History passes self.convergence at construction; the three
Game::*_with_arena callsites in time_slice.rs read it. Also renames
TimeSlice::convergence the method (now iterate_to_convergence) to
disambiguate from the new field.
Pure plumbing — no new public API, no behavioral change for users on
default options. Makes Damped EP reachable through the History path.
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.
Inherent method that applies α-damping to the outgoing message via
Gaussian::damp_natural. The Factor trait impl delegates with α=1.0,
preserving today's behavior bit-equal. Variable write switched from
`trunc` to `cavity * damped` — algebraically identical when α=1.0
(cavity * new_msg = trunc by construction); reflects partial-update
math when α<1.0.
Adds an EP damping coefficient defaulting to 1.0 (undamped). Will be
read by run_chain in a follow-up commit. By itself this commit changes
no behavior — existing constructors using ..Default::default() pick up
the new field automatically.
Computes α·new + (1−α)·self in natural-parameter space. Will be used
by TruncFactor and MarginFactor to support opt-in EP damping via
ConvergenceOptions::alpha.
Six tasks: Gaussian::damp_natural helper, ConvergenceOptions::alpha
field, TruncFactor and MarginFactor propagate_with_alpha pair, DiffFactor
+ Game integration (the big task — must land atomically), and
end-to-end tests for max_iter and alpha behavior.
Smallest-scope realisation of spec §"Built-in schedules" Damped: a
ConvergenceOptions::alpha field plumbed through run_chain to a new
Gaussian::damp_natural helper applied inside TruncFactor and
MarginFactor's propagate. alpha=1.0 default keeps every existing
golden bit-equal; alpha<1.0 stabilises oscillating fixed-point loops
on hard graphs.
Defers Schedule trait integration, nat-param convergence switch,
oscillation auto-detect, Residual/OneShot, and Synergy/ScoreFactor —
each gets its own future 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.
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.
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.
Three independent cleanups: dedupe Game::likelihoods and likelihoods_scored
via a run_chain helper taking a make_link closure, make BuiltinFactor's
log_evidence match exhaustive, and fix stale numerics in the T4 plan doc.
Implements T3 of `docs/superpowers/specs/2026-04-23-trueskill-engine-redesign-design.md` Section 6. Plan: `docs/superpowers/plans/2026-04-24-t3-concurrency.md` (11 tasks).
## Summary
### Breaking
- `Send + Sync` bounds added to public traits: `Time`, `Drift<T>`, `Observer<T>`, `Factor`, `Schedule`. All built-in impls satisfy these via auto-derive; downstream custom impls 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` infrastructure (`src/color_group.rs`) with greedy graph coloring. 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
### Sequential (no rayon, default build)
| Metric | Before T3 | After T3 | Delta |
|---|---|---|---|
| `Batch::iteration` | 22.88 µs | 23.23 µs | **+1.5%** (noise) |
| `Gaussian::*` | ≈218–264 ps | ≈236 ps | within noise |
**No sequential regression.** Default build is as fast as T2.
### Parallel (`--features rayon`, Apple M5 Pro, auto thread count)
| Workload | Sequential | Parallel | Speedup |
|---|---:|---:|---:|
| 500 events / 100 competitors / 10 per slice | 4.03 ms | 4.24 ms | **1.0×** |
| 2000 events / 200 competitors / 20 per slice | 20.18 ms | 19.82 ms | **1.0×** |
| 5000 events / 50000 competitors / 1 slice | 11.88 ms | 9.10 ms | **1.3×** |
### ⚠️ The spec's >=2× target was not met on realistic workloads.
T3's within-slice color-group parallelism only shows material benefit when a slice holds many events AND the competitor pool is large enough to give the greedy coloring room to partition. Typical TrueSkill workloads (tens of events per slice) don't fit that profile — 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 and would deliver the spec's ~50–500× online-add speedup. Deferred to a future tier.
## Determinism
`tests/determinism.rs` runs a 200-event history at thread counts {1, 2, 4, 8} via `rayon::ThreadPoolBuilder::install` and asserts every `(time, posterior)` pair has bit-identical `mu` and `sigma` (compared via `f64::to_bits()`). Passes.
## Internals
- 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`), guaranteed by construction in `TimeSlice::recompute_color_groups`. Sequential path unchanged from T2.
- `RAYON_THRESHOLD = 64` — color groups smaller than this fall back to sequential inside `sweep_color_groups` to avoid task-spawn overhead.
- Thread-local `ScratchArena` per rayon worker thread.
## Test plan
- [x] `cargo test --features approx` — 96 tests pass (74 lib + 22 integration)
- [x] `cargo test --features approx,rayon` — 97 tests pass (+1 determinism)
- [x] `cargo clippy --all-targets --features approx -- -D warnings` — clean
- [x] `cargo clippy --all-targets --features approx,rayon -- -D warnings` — clean
- [x] `cargo +nightly fmt --check` — clean
- [x] `cargo bench --bench batch --features approx` — 23.23 µs (no regression vs T2)
- [x] `cargo bench --bench history_converge --features approx,rayon` — runs on all three workloads
- [x] Bit-identical posteriors across `RAYON_NUM_THREADS={1, 2, 4, 8}` — verified
## Commit history
13 commits on `t3-concurrency`. Each task is self-contained and bisectable. See `git log main..t3-concurrency` for the full list.
## Deferred
- **Cross-slice parallelism** (dirty-bit slice skipping) — the path that would actually speed up typical TrueSkill workloads.
- **Default-on `rayon` feature** — spec called for default-on; we keep it opt-in until the feature proves stable in production use.
- **Synchronous-EP schedule with barrier merge** — alternative parallel strategy per spec Section 6.
- **`MarginFactor` / `Outcome::Scored`** — T4.
- **`Damped` / `Residual` schedules** — T4.
- **N-team `predict_outcome`** — T4.
- **`Game::custom` full ergonomics** — T4.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Reviewed-on: #2
Co-authored-by: Anders Olsson <anders.e.olsson@gmail.com>
Co-committed-by: Anders Olsson <anders.e.olsson@gmail.com>
logaritmiskandClaude Opus 5