dc1f4d5847a2d5c3309192fd6152f71423a1f592
203
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  logaritmiskandClaude Opus 5
|
8116fd081f |
test: localise the erfc_inv tail residual to the caller's argument
Scouting crates.io for a more accurate `erfc` than `libm` turned up a 1.16e-12 relative error in `erfc_inv` at `p_draw = 0.999999`, measured against a 70-digit `decimal` reference. It looked like too few Newton steps. It is not: adding a fourth changed nothing. The error is in forming the argument. `1.0 - 0.999999` is `1.0000000000287557e-06` — 0.999999 is not representable, and subtracting from one cancels, leaving 2.9e-11 of relative error before `erfc_inv` is entered. Given an exactly-representable argument it returns 1.8e-16. So the routine was never the problem, and the extra iteration has been reverted rather than shipped as a fix for a defect that was not there. `puruspe::inverfc` returns the identical wrong value for the identical reason, which is what makes the shared upstream cause obvious. Adds a test that separates the two, and corrects a quantile constant in `erfc_inv_matches_known_quantiles` that was recalled rather than computed: `Phi^-1(0.9999995)` is 4.89163847569859, not 4.891638475699099. The others were checked against the same reference and were right. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
17d072b2ae |
fix: route every transcendental through libm, and combine sigmas with hypot
Follow-on from #41, which added `libm` for `erfc`. Surveying what else the dependency offers: its unique surface over `std` is `erf`/`erfc`, `lgamma`/`tgamma` and Bessel functions, and only the first was ever needed. But the survey found something better than another special function. `std`'s `exp` and `ln` delegate to the *system* math library. IEEE 754 specifies the basic operations and `sqrt` exactly and says nothing about transcendentals, so those differ per platform. Measured here over 200k inputs: exp: 19425/200000 differ from libm (worst 1 ulp) log: 9932/200000 differ Inference is an iterative fixed point, so a one-ULP difference can change an iteration count and move the answer by more than one ULP. Routing every transcendental through `libm` makes a fit reproducible across platforms — a stronger guarantee than `tests/determinism.rs`, which only covers thread counts. It costs nothing. `Batch::iteration` measured -2.7% [-5.7%, -0.3%] with the whole set swapped, and not one golden moved. Also switches the two places that combined sigmas as `sqrt(a^2 + b^2)` to `hypot`. Squaring overflows to infinity above ~1.3e154 and flushes to zero below ~1.5e-154 — measured, the naive form returns `inf` where `hypot` returns 1.41e160 — and `Gaussian`'s constructors are public, so a caller can reach both ends. Deliberately not done: rewriting the KL divergence's `ln` of a ratio via `ln_1p`. The cancellation is real as the ratio approaches one, but measured absolute error is at most ~1e-11 in a quantity of order 0.4 nats, so it changes nothing. The invariant is recorded in `CLAUDE.md` and on `erfc`'s own docs, since nothing enforces it mechanically. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
3dd659307a |
fix: replace the erfc approximation with libm, for free
#41 asked whether the Numerical Recipes `erfcc` approximation — 1.2e-7 relative, and the binding accuracy constraint on the whole crate — was worth replacing, given it sits in the inference hot loop. It is, and it costs nothing. Measured, against an independent incomplete-gamma reference: range previous (NR) libm [-3, 0] 7.95e-8 2.15e-14 [0, 0.5] 8.69e-8 4.70e-14 [0.5, 2] 9.38e-8 1.24e-12 [2, 6] 1.04e-7 5.20e-14 [6, 26] 1.07e-7 1.75e-13 erfc(0) 1.00000003 1.0 (exactly) |erfc(z)+erfc(-z)-2| 6.00e-8 2.22e-16 Performance, on `benches/batch.rs`: change [-3.34% +2.26%], p = 0.89 — no change detected. That result is counterintuitive, because libm's erfc is 1.65x slower when swept uniformly over [-2.5, 2.5]. The sweep was the wrong input distribution. Capturing the arguments inference actually passes: |x|<0.5 96.16% 0.5-0.84 2.05% 0.84-1.25 1.24% 1.25-2 0.54% 2-6 0.00% 98% fall below 0.84375, which is exactly where FDLIBM skips the exponential entirely — while the NR form always pays for one. On the real trace libm is the faster of the two (3.23 vs 3.78 ns/call). An ad-hoc `Instant` harness reported a 16% end-to-end speedup; that was an artifact of its own setup allocating and leaking per run, and criterion's verdict of "no change" is the one to believe. What it bought: - `compute_margin` against exact quantiles: 8.4e-8 -> 1.7e-16. - `cdf(mu, mu, sigma)` is now exactly 0.5; it was 1.5e-8 out. - `sf + cdf` sums to one within a ULP, from 3e-8. - `erfcx`'s two branches now agree to round-off across the crossover rather than to 1e-7, so the log-space evidence path and the linear one are consistent. - Ten test tolerances tightened from 1e-6 to 1e-13..1e-15, and the prediction floor is now the integrator's rather than `cdf`'s. Five goldens moved, by 2.4e-9 to 6e-7 — the magnitude of the removed error, and `test_env_ttt`'s mu still rounds to the same six decimals. Re-recorded with more digits so future drift stays visible. Verified as movement toward truth per the goldens policy: every value now derives from a primitive checked against an independent reference and satisfying the exact identities, which the previous one did not. Adds `libm` — zero transitive dependencies, rust-lang maintained. Closes #41 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmisk
|
7e289ee834 | chore: Release trueskill-tt version 0.4.1 v0.4.1 | ||
|
logaritmiskandClaude Opus 5
|
564969ee5d |
test: pin quality()'s N-group closed form, closing the README cross-check
The scan for precision defects found none in `quality()` — but it did find that N identical teams have an exact closed form, which is a much stronger regression net than the single two-team golden that was there. For two identical single-player teams quality is `sqrt(2b^2 / (2b^2 + s1^2 + s2^2))`. With the conventional parameters that ratio is exactly 1/5, and the N-group generalisation is `(1/5)^((n-1)/2)` — one factor per adjacent pair. Measured across n = 2..10 the implementation matches to 1e-9, so the determinant path that #9 rebuilt is correct over the whole range, not just at n = 2. The n=3 and n=5 values (0.200 and 0.040) are also what the `trueskill` Python package produces for the same configuration, which is the cross-implementation check the README Todo has been asking for since the redesign. Asserted separately as literals so a change to the closed-form reasoning cannot silently carry them along. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
683813ec10 |
fix: correct erfc_inv's sign error and keep evidence in log space
A systematic scan for precision defects, following the tail-precision work in |
||
|
logaritmisk
|
2a48d10aa9 | chore: Release trueskill-tt version 0.4.0 v0.4.0 | ||
|
logaritmiskandClaude Opus 5
|
d4f91fd221 |
fix: reject convergence options that silently disable inference
`Game::ranked` and `Game::scored` validated `p_draw` and `score_sigma`
but never `convergence`. `ConvergenceOptions` has public fields and
`GameOptions` carries one, so a caller could hand the engine a set that
`HistoryBuilder`'s eager asserts never saw. Past that, the only guard
was a `debug_assert!`, which is gone in the profile users ship.
An `alpha` of zero is the bad case, and it fails silently rather than
loudly. Measured in release before the fix:
likelihoods: [[Gaussian { pi: 0.0, tau: 0.0 }],
[Gaussian { pi: 0.0, tau: 0.0 }]]
Every EP update unapplied, every likelihood uninformative, inference
returning the priors it was given — and an `OwnedGame` that looks
entirely ordinary to the caller. `HistoryBuilder::convergence` already
documents exactly this hazard; the `Game` constructors just did not
share the check.
Adds `ConvergenceOptions::validate`, called by both constructors.
Rejects `alpha` outside `(0.0, 1.0]` and negative `epsilon`; NaN fails
both comparisons and is rejected too.
`tests/validation.rs` states the release-mode guarantee for the whole
public surface, not just this hole, and CI already runs the suite in
release. Probing the other conditions #18 lists found five of eight
already enforced — ties without a draw probability, per-event score
sigma, weight/team dimensions, draw-probability range, score-sigma
range — so this closes the remaining gap rather than the whole issue.
The engine keeps its `debug_assert!`s as invariant documentation.
Refs #18
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
|
||
|
logaritmiskandClaude Opus 5
|
8c087ad015 |
fix!: apply competitor configuration whenever it is supplied
`Member::with_prior` and `with_drift_scale` were consumed only on the branch that *creates* a competitor — `priors.remove` sat inside `if !self.agents.contains(..)`. Supplying either for a key the history already knew did nothing at all: no error, no warning, and output computed from the default prior. A prior applied on a competitor's very first event and was silently discarded ever after. Configuration now applies whenever supplied. Two details this forced: Configuration is tracked per *field* rather than as a merged `Rating`. A member setting only `drift_scale` must not also assert the default prior, or it would silently undo a prior seeded on an earlier event. Slice state has to be refreshed. `drift_scale` is re-derived on every forward pass, but a prior is written into the competitor's earliest slice once, at ingestion, and `iteration` refreshes only slices after the first. Without the refresh a late prior would reach the drift terms and nothing else — a subtler version of the drop being fixed. This was caught by a test, not by reading the code. Conflicting values for one competitor within a single batch are now `ConflictingCompetitorConfig` rather than resolved by iteration order. Events in a batch are unordered, so "last one wins" would make the result depend on traversal — and `tests/ingestion_equivalence.rs` exists to rule exactly that out. Repeating the same value stays inert, which is the shape callers get when configuration is a property of the domain. That invariant turned out to be tested only for *unconfigured* competitors: every helper in that file built members with `Member::new`. Extended to cover configured ones, including a check that configuration changes the fit at all, so the order tests cannot pass vacuously. `with_prior` had no coverage under `tests/` whatsoever, which is how this survived. Adds `tests/competitor_config.rs`. `drift_scale_is_ignored_after_first_appearance` asserted the old behaviour and now asserts the new one. It was written as a deliberate change-detector — "moving the capture would be a visible break, not a silent one" — so it inverted rather than being deleted. Also removes `InferenceError::ConvergenceFailed` and `NegativePrecision`, which no code path ever constructed: public variants advertising failure modes no caller could observe. Partial #20 — its other items were already resolved, except `Outcome::winner` still panicking. BREAKING CHANGE: `prior` and `drift_scale` now take effect for competitors the history already knows, where they were previously ignored; a batch supplying conflicting values for one competitor is now an error. `InferenceError::ConvergenceFailed` and `InferenceError::NegativePrecision` are removed. Closes #10. Refs #20. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
7341669d1a |
fix: stop destroying tail precision in evidence and truncation
`erfc` is sound — it holds ~1e-7 *relative* accuracy down to 1e-296 with no tail degradation. Three expressions built on it threw that away by subtracting quantities that both approach the same value. 1. `cavity_evidence` computed `1.0 - cdf(margin, ..)`, which is algebraically `sf(margin, ..)` and numerically a catastrophe: 7% error by eight sigma, and exactly zero past ~8.3, where the true probability is 1e-19 and perfectly representable. Clamped, that reached `log_evidence` as ln(f64::MIN_POSITIVE) = -708 whatever the truth was — off by 665 nats at nine sigma. `1 - cdf` is smallest precisely when the result contradicts the prior, so the model-comparison number was worst for upsets: the observation it exists to notice. Adds `sf`, the survival function, computed without the subtraction. The tie branch picks whichever tail keeps both of its terms small, for the same reason. 2. `v_w` computed the inverse Mills ratio as `pdf(-a) / cdf(-a)`. Both underflow together past about 39 sigma, giving `0 / 0` and putting NaN straight into the posterior. Adds `erfcx`, so the shared `exp(-alpha^2 / 2)` cancels analytically instead of being evaluated twice and divided. 3. With that fixed, `w = v * (v - alpha)` became the next casualty: `v` tends to `alpha`, so the gap lost every digit and drove `w` above 1, making `sqrt(1 - w)` NaN at alpha = 1e6. The gap now comes from its asymptotic series, which forms no difference at all. The tie branch had the same defect one expression over — `v * v - u` with both terms at 1e18 returned w = -128 — and a far-tail window is indistinguishable from a half-line, so it shares the asymptotic. No public signature changes, and no existing golden moved: every one of these only alters regions the old code got wrong. The two identity tests are asserted at 1e-6 rather than tighter because `erfc` is not exactly antisymmetric — `erfc(z) + erfc(-z)` differs from 2 by ~3e-8, and `erfc(0)` returns 1.00000003. That floor is tracked separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
2fff745c3b |
feat: let observers be shared, boxed, or borrowed
`History` takes its observer by value and never hands it back, so a
caller who wanted to read what an observer recorded had no way to keep a
handle to it. The natural spelling did not compile:
let recorder = Arc::new(Recorder::default());
History::builder().observer(Arc::clone(&recorder))
// error[E0277]: `Arc<Recorder>: Observer<i64>` is not satisfied
The workaround was for every observer to wrap each of its own fields in
an `Arc` and derive `Clone` — one allocation and one lock per field, a
pattern each implementor had to rediscover, and nothing documenting it.
Adds blanket `Observer` impls for `Arc<O>`, `Box<O>` and `&O`. All are
`?Sized`, so `Arc<dyn Observer<T>>` and `Box<dyn Observer<T>>` work too
and an observer can be chosen at runtime. Also adds
`History::observer()` and `into_observer()`, so a non-shared observer's
state can be inspected in place or reclaimed after `converge` without
needing interior mutability at all.
`tests/observer.rs` is simplified to the shared spelling, so the
recommended pattern is the one demonstrated rather than the workaround.
Closes #40
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
|
||
|
logaritmiskandClaude Opus 5
|
3c2f9ac64c |
feat: add expected information gain for active matchup selection
`quality()` answers "is this matchup fair". Callers picking which
comparison to run next need "is this matchup informative", and the two
coincide only for two evenly matched competitors. Without a principled
alternative, downstream code was reaching for hand-rolled heuristics
like `quality * sigma_a^2 * sigma_b^2`, which double-counts uncertainty:
the two factors are not independent.
Adds `expected_information_gain`, the outcome-weighted divergence
between current beliefs and the beliefs each result would produce:
EIG = SUM P(outcome) * KL(posterior_after(outcome) || prior)
Available standalone over `Rating`s, and as
`History::expected_information_gain` using current skills and the
history's own beta, drift and p_draw — so the outcomes it weighs are the
ones that would actually be fitted.
This is the mutual information between the outcome and the skills, which
gives an analytic ceiling: gain cannot exceed the entropy of the thing
being observed, so at most `ln k` nats for k outcomes. That bound is the
sharpest test available, because an acquisition function is unusually
exposed to returning finite, plausible, monotone numbers while being
wrong — it would simply select slightly worse matchups forever. A
prototype of this returned 4.77 nats from a sign error while passing
every monotonicity check; `never_exceeds_the_entropy_of_the_outcome`
catches that class unconditionally.
Measured against the ceiling the values are meaningful rather than
vacuous: 0.382 nats for an even matchup between diffuse priors against
an 0.693 ceiling, falling to 0.013 for a lopsided one and 0.000 for a
hopeless one.
`disagrees_with_the_quality_times_variance_heuristic` pins down that
this is not a monotone transform of the heuristic it replaces — the two
rank a lopsided matchup and a confident even one in opposite orders — so
a later "simplification" cannot quietly revert to it.
Cost is one inference pass per possible outcome, documented on the
public API alongside the shortlist-then-score pattern, so callers do not
discover it in production.
Also folds the duplicated key-gathering in `predict_quality` and
`performances` into one validated `member_skills`.
Refs #39
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
|
||
|
logaritmiskandClaude Opus 5
|
507894dae7 |
refactor!: close the remaining API gaps from #21
Three unrelated small defects, all requiring signature changes: - `Game::one_v_one` hardcoded `GameOptions::default()`, so a 1v1 could never set `p_draw` or convergence options — and a drawn 1v1 was therefore unreachable through it, since the default `p_draw` is zero. It now takes `&GameOptions` like every other constructor. - `Observer::on_batch_processed` was declared on the trait and never called from anywhere: implementors wired up a callback that could not fire. It is now called after each slice sweep, and renamed `on_slice_processed` to match the vocabulary the codebase adopted in T2 — the unit of work is a `TimeSlice`, not a batch. A slice is swept once travelling backward and once forward, so a multi-slice history fires it twice per slice per iteration; the doc comment says so. - `pub mod factors` sat beside `pub(crate) mod factor`, two module paths differing by one character with only one of them importable. The public facade is now `graph`. Tests cover each as a behaviour rather than a compile check: a drawn 1v1 succeeds only when p_draw is supplied, and the observer tests fail if any callback stops firing. BREAKING CHANGE: `Game::one_v_one` takes a fourth `&GameOptions` argument; `Observer::on_batch_processed` is renamed `on_slice_processed`; the `factors` module is renamed `graph`. Closes #21 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
bb2a845882 |
feat!: N-team outcome prediction with draw mass, replacing the 2-team panic
`predict_outcome` asserted `teams.len() == 2` and returned `[p, 1 - p]`, allocating no probability to a draw even with `p_draw > 0`. For a draw-enabled model the numbers were simply wrong, at any team count. It now returns `Result<Prediction, InferenceError>` and supports N teams. Two algorithms, both deterministic: - Who finishes first. Performances are independent Gaussians, so this separates into a one-dimensional integral per team rather than a multivariate orthant probability. Adaptive Gauss-Kronrod evaluates it to ~1e-15, matching the exact two-team closed form. - A specific finishing order. The factor graph only constrains rank-adjacent teams, so a full order is a chain of local constraints, not a general orthant integral. That chain collapses into a sequential recursion over cumulative integrals: O(teams * grid) per order. Fixed-node Gauss-Hermite is the obvious tool for the first and is a trap: when a rival's sigma is small the CDF product becomes a step narrower than the node spacing, and the nodes step over it. Measured 4.4e-4 off the closed form on a mildly skewed matchup and 1.7e-2 on a small-sigma one, while still returning something that looks like a probability. Adaptive refinement is what makes that case safe, and `win_probabilities_survive_a_rival_with_a_tiny_sigma` pins it down. The acceptance test is an identity rather than a golden: the outcome space is exhaustive and disjoint, so the probabilities sum to one. Any drift is integration error and nothing else. Gauss-Hermite failed it at 4.4e-4; this holds to ~1e-9. Also from #21: unknown keys are now reported rather than dropped, so a team of strangers can no longer produce a confident-looking prediction. `predict_quality` returns `Result` for the same reason. BREAKING CHANGE: `predict_outcome` returns `Result<Prediction, _>` instead of `Vec<f64>`; `predict_quality` returns `Result<f64, _>`. Refs #21, #39 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
87fca8dcca |
docs: correct drifted documentation and compile the README in CI
Four README code blocks no longer compiled: `Player` was renamed `Rating`
in T2, the `Drift` trait gained a `T: Time` parameter and a second method,
and two blocks were missing imports outright. The `Rating` example needed
more than a rename — with the binding unused, `T` is ambiguous because
`ConstantDrift` implements `Drift<T>` for every `T`, so it now carries an
explicit annotation.
Nothing compiled those blocks. `src/lib.rs` gains a `cfg(doctest)` struct
carrying `#[doc = include_str!("../README.md")]`, which turns every `rust`
block into a doctest without displacing the curated crate docs as the
front page. Verified it bites: reintroducing `Player` fails the build with
E0432 rather than shipping. Illustrative blocks are fenced `text` — note
that a bare fence defaults to `rust` under rustdoc, which is how the
`variance_delta = elapsed * γ²` formula became a compile error.
Prose fixes: README claimed `Gaussian::forget` takes a square root (it
works in variance space) and pointed at a `.gamma()` builder method that
does not exist. CLAUDE.md's data-flow diagram spliced the public ingestion
shape into the internal one — `Team` is not in that chain — listed
`cdf()`/`erfc()` as public when they are `pub(crate)` and private, and
called `SkillStore` public when only `CompetitorStore` escapes the crate.
Rustdoc fixes: `EventBuilder::scores_with_sigma` claimed a debug-assert
that `Outcome::scores_with_sigma` never had and whose own docs contradict;
rejection happens at ingestion as `InvalidParameter`. `event.rs` described
`add_events_with_prior` as replaced when it is still the ingestion
chokepoint. `factors.rs` advertised `Game::custom` without noting it is
`#[doc(hidden)]`. Internal T2/T4 milestone labels are dropped from public
items; the ones in the private `time_slice` module are left alone.
Closes #35
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014b6wy2q8rnFK8U8GPJVQNU
|
||
|
logaritmiskandClaude Opus 5
|
ef62b57a08 |
fix(release): skip the changelog hook during a dry run
`just release-plan` is documented as a preview that writes nothing, but cargo-release runs pre-release hooks during a dry run too. The hook wrote CHANGELOG.md and `git add`ed it, so the clean-tree check in `just release` then refused to run — the repo's own two-step release workflow could not be followed as written. Guard the hook on DRY_RUN, which cargo-release 1.1.5 exports to the hook environment (verified by dumping `env` from a throwaway hook; it also sets CRATE_NAME, PREV_VERSION and NEW_VERSION). The clean-tree check itself is left alone: it is load-bearing, because publishing is irreversible. Closes #36 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014b6wy2q8rnFK8U8GPJVQNU |
||
|
logaritmisk
|
b2a7ade10c | chore: Release trueskill-tt version 0.3.0 v0.3.0 | ||
|
logaritmiskandClaude Opus 5
|
617bc07f6f |
feat: allow drift to vary per competitor via Member::with_drift_scale
Drift was a property of the History, so every competitor drifted at the same rate and a fixed reference point could not share a graph with moving competitors. A bot at a known strength, a rating floor, a course difficulty — all of them drifted along with the players. Member::with_drift_scale(s) multiplies the drift *variance* a competitor accumulates, so s is in the same units as gamma: ConstantDrift(g) at scale s behaves exactly as ConstantDrift(g * s) would for that competitor. A scalar rather than a per-competitor Drift keeps History's single D type parameter untouched and stays Copy. 0.0 pins a competitor still. The scale lives on Rating, beside the drift it scales, and is applied only through Rating::drift_variance_delta / drift_variance_for_elapsed. Making those the sole entry points means a caller cannot reach the raw drift and silently skip a competitor's scale — the filtered pass was exactly that bug during development, caught because its test was written before the wiring. Like with_prior, the scale is competitor configuration captured at first appearance rather than a per-event override; a competitor that is static is static, and a scale that changed between events would make the skill trajectory hard to interpret. Member's docs claimed prior was a per-event override, which the code has never done — corrected here. A negative scale is rejected rather than squared into its absolute value, and a non-finite one rejected outright, both as InvalidParameter. None means 1.0, so no existing call site changes and no existing fit moves. Adding a public field to Member does break struct-literal construction downstream. Closes #34 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014b6wy2q8rnFK8U8GPJVQNU |
||
|
logaritmiskandClaude Opus 5
|
1a88678384 |
refactor!: remove ConvergenceReport::slices_skipped
Closes #33. The field was public, hardcoded to 0 at both construction sites, and had no route to ever being non-zero. It was added in T3 as the reporting surface for dirty-bit slice skipping. That feature is #4, closed as unworkable — the ceiling measured at ~6% against a projected 5-50x, on top of three independent soundness blockers. #32, which reattributed the cost to ingestion, is closed too: the re-convergence is necessary work rather than waste, because appending one event genuinely moves the involved competitors ~1.2 sigma across their whole history. Nothing left would ever populate it. This is the same defect class as #19, where this arc started: a public surface that looks implemented, reports a plausible value, and is inert. A caller reading `slices_skipped: 0` reasonably concludes "no slices were skipped this run", not "this feature does not exist". Removed rather than documented as reserved. Its only value was as a hook for a plan that no longer exists, and keeping it preserves the shape of that plan. Breaking, but ConvergenceReport is returned rather than constructed by callers, so the only breakage is code reading a constant zero. Also added a test asserting every remaining field carries real information — iterations non-zero, final_step finite, log_evidence a finite negative log probability, and per_iteration_time holding one duration per iteration. Mutation-proved: pinning per_iteration_time to an empty SmallVec fails it. The next always-constant member now has to survive an assertion rather than just a reviewer's attention, which is the actual lesson of #19 and #33. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc |
||
|
logaritmisk
|
5f46296671 |
chore: ignore proptest regression seed files
proptest writes tests/*.proptest-regressions when a property fails, seeding a replay of that exact case. Useful locally; noise in the repo when the failure came from a deliberate mutation rather than a real defect. |
||
|
logaritmiskandClaude Opus 5
|
2745fbb622 |
test: add property-based tests, a shared finiteness helper, and boundary inputs
Most of what remained on #26. **Property tests (`tests/properties.rs`, proptest as a dev-dependency).** Four invariants over generated 1v1 schedules rather than hand-written fixtures, which is where this crate's shipped defects actually hid — a linear evidence product that underflowed only past ~1000 teams, and a batching path no golden exercised because every golden ingests in one call: - converged posteriors are always finite with positive sigma - log-evidence, batch and filtered, is finite and never above zero - filtered evidence is invariant to whether `converge` has run - one-at-a-time ingestion reaches the same fixed point as batched The invariance property was mutation-proved: making `filtered_step` read `skill.forward` instead of the carried message fails it with `-1.1038430064192069 -> -1.1135747072822761`. **Shared finiteness helper (`tests/common/mod.rs`).** `assert_finite` was local to `degenerate_inputs.rs`. It now also rejects a non-positive sigma, which the old version let through — `Gaussian::sigma` reports a non-positive precision as improper rather than trapping, so a collapsed posterior would have passed a finite-only check. **Boundary inputs.** Zero and negative weights, out-of-order timestamps, and extreme beta/sigma combinations. Worth recording that zero weight reaches `(m - performance.exclude(..)) * (1.0 / w)` — a division by zero — and the posterior comes out finite anyway; the test pins that rather than asserting what ought to happen. The weight tests `expect()` the commit rather than returning early on error, because an early return would have made them vacuous the moment validation changed. I checked that specifically by turning the return into a failure and confirming it did not fire. Not done, and left on #26: benchmark regression gating. Nothing fails on a regression today; making it fail needs a threshold chosen against how noisy the shared runner is, which is a policy call rather than a mechanical one. 60 test binaries, up from 56. MSRV 1.85 verified with proptest in the graph. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc |
||
|
logaritmiskandClaude Opus 5
|
6d2573b92e |
perf: make the per-slice SkillStore compact instead of dense
Closes #17. Each `TimeSlice` owned a `Vec<Skill>` indexed by the GLOBAL `Index.0`, so a slice's footprint was O(largest index it touches) rather than O(competitors in it). Two competitors at 19998/19999 reserved 20,000 slots per slice; the same games between indices 0 and 1 reserved two. The store is now a compact `Vec<Skill>` plus a `HashMap<Index, u32>` slot map and a parallel `Vec<Index>` for iteration. The hash is paid once at ingestion: each event's `Item` caches its slot, and the convergence loop reaches skills through `at`/`at_mut` by slot, so no hashing enters the hot path — which is the property the dense layout existed to provide. Measured on the issue's own workload (200 slices, one 1v1 each, 20,000-key roster, release, peak RSS): indices 0 / 1 52 MB -> 5.55 MB indices 19998 / 19999 309 MB -> 8.39 MB The 257 MB gap is now 2.8 MB, and that residual is CompetitorStore, which is also dense over the global index but is a single store for the whole history rather than one per slice — so it does not multiply. Left alone deliberately. Benchmarks, against the pre-change code: Batch::iteration +2.4% (regressed) history_converge x3 -18.8%, -21.6%, -21.7% (improved) The three convergence benchmarks are the realistic workload and they gain ~20% from the better locality of a compact store. The micro-benchmark loses 2.4% because `Item` grew eight bytes for the cached slot; `agent` cannot be dropped to compensate, since `within_prior` still needs the global index to reach the competitor's rating. I judged 2.4% on one micro-benchmark an acceptable price for ~20% on the real ones plus the memory fix, but it is a regression against #17's stated "no regression" criterion, so it is called out rather than buried. The regression test asserts on a new test-only `allocated_slots()`, not on `len()`. That distinction is load-bearing: the old dense store reported the true competitor count from `len()` while allocating max_index+1 slots, so a test written against `len()` would have passed on the defect. Mutation-proved by re-adding the dense padding, which fails it. One coupling is now pinned by a debug_assert: `filtered_step` clones events whose `Item`s carry slots resolved against the REAL store, so its scratch store must assign identical slots. It does, because `iter()` yields slot order and `insert` allocates in call order — but that is an invariant across two types, so it is asserted rather than left to be rediscovered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc |
||
|
logaritmiskandClaude Opus 5
|
1ac3b21db5 |
fix: enforce EventBuilder weight/team length in release
Part of #18. `EventBuilder::weights` guarded the length match with a `debug_assert!`, so release builds accepted a mismatch, silently dropped the weights, and ingested the event anyway. That is the exact shape #18 is about: validation that exists only where it is least needed. The setters return `Self` to keep the chain fluent, so they cannot return a `Result`. The builder now records the first failure and `commit` returns it as `MismatchedShape`. The weights are not applied on mismatch either, so a partially-weighted team cannot reach the history by another route. Two tests in tests/degenerate_inputs.rs, whose CI job runs in release — which is the only place the old behaviour differed. The second test needed strengthening before it was worth anything. As first written it committed a ONE-team event, which ingestion rejects for an unrelated reason, so it passed under a mutation that disabled the whole check. It now uses two teams, so ingestion would otherwise succeed and the assertion is actually load-bearing. Both tests were then mutation-proved together: disabling the error path in `commit` fails both in release. #18 stays open. The remaining debug_asserts live in `ranked_with_arena` and `scored_with_arena`, and promoting those means threading `Result` up through `Event::compute`, `TimeSlice::iteration`, `log_evidence` and `filtered_step` — which lands on the public API as `log_evidence() -> Result<f64>` and `filtered_learning_curve() -> Result<...>`. That is a trade-off about what the query API should look like, not a mechanical change, so it is not mine to decide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc |
||
|
logaritmiskandClaude Opus 5
|
4e043364fd |
perf: stop cloning inference inputs in OwnedGame and ingestion
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 |
||
|
logaritmiskandClaude Opus 5
|
aff3fb948d |
refactor!: replace emptiness-as-sentinel with Option for results and weights
Third of the five remaining items on #23. `add_events_with_prior` and `TimeSlice::add_events` took `Vec<Vec<f64>>` and `Vec<Vec<Vec<f64>>>` where an empty vec meant "not supplied" — so an empty outer vec and a genuinely empty event list were the same value, and every reader had to know which. Both are now `Option`, and the "not supplied" branches read as `None` arms rather than `is_empty()` checks. Two things fell out of the change that a sentinel would have hidden: The tie pre-check iterated `results` directly. Under `Option` it needs `.iter().flatten()`, which makes explicit that a `None` results list has no ties to reject — previously an empty vec silently skipped the same loop and looked identical to "checked, found nothing". `MismatchedShape.got` could no longer be `results.len()`, because at the point of the error there may be no vec to take a length from. It is now computed as `map_or(0, Vec::len)` before the error is built. MSRV note: my first draft used let-chains for the two validations. Those need Rust 1.88 and this crate pins 1.85 — it compiled locally on 1.98 and would have failed only in the MSRV CI job. Rewritten with `is_some_and`, and verified by installing 1.85.0 and building against it rather than by assuming the removal was complete. Breaking: `TimeSlice::add_events` is public. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc |
||
|
logaritmiskandClaude Opus 5
|
06ed24b240 |
refactor!: make Competitor::message an Option, and compute_elapsed loud
Two of the five remaining items on #23. `Competitor.message` was a `Gaussian` using the improper `N_INF` as an "unset" sentinel, so `message != N_INF` meant "has a message" and every reader had to know that convention. It is now `Option<Gaussian>`, which makes "no message yet" and "a legitimately improper message" distinguishable at the type level instead of by float comparison. Worth noting what the change surfaced: switching the type turned every read site into a compile error, and there were eight — two in the convergence sweep, five in ingestion, one in new_backward_info. The last is the interesting one: `skill.backward = agents[agent].message` needed `unwrap_or(N_INF)` rather than an unwrap, because an absent message genuinely does mean the improper identity there. A sentinel-based refactor would have had to find that by reading. This is a breaking change: `message` is a public field. It rides the next minor bump. `compute_elapsed` clamped a negative elapsed to zero silently. Negative elapsed means slices are being visited out of time order, which would otherwise make drift *reduce* uncertainty. Release still clamps, so a bad timestamp degrades to "no drift" rather than corrupting a posterior, but debug now trips — getting there is a slice-ordering bug, not something callers can cause with ordinary data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc |
||
|
logaritmiskandClaude Opus 5
|
9b2c2b38c8 |
docs: complete the public API documentation contract
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 |
||
|
logaritmisk
|
07285283b6 | chore: Release trueskill-tt version 0.2.0 v0.2.0 | ||
|
logaritmiskandClaude Opus 5
|
b73cf0145a |
chore: dual-license MIT OR Apache-2.0
The Rust ecosystem convention, and what kickscore, xy and saphyr already use —
kickscore being the closest sibling to this crate.
LICENSE-APACHE is the canonical 201-line Apache-2.0 text with the appendix
left as the unfilled template, which is the form Rust crates ship. LICENSE-MIT
carries the copyright line. Both are picked up by cargo automatically and
appear in the packaged crate.
This also unblocks the release workflow. `cargo publish` does not check the
license field when the target is an alternative registry, but cargo-release
does, and refuses outright:
error: trueskill-tt is missing the following fields:
license || license-file
--no-verify does not bypass it. So `just release` was inert without this,
whatever cargo publish alone would have accepted.
README gains the conventional dual-license section and the contribution note
that dedicates inbound contributions under the same terms.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc
|
||
|
logaritmiskandClaude Opus 5
|
56ff01074f |
docs(cargo): correct the licence note — kellnr does not require one
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 |
||
|
logaritmiskandClaude Opus 5
|
8d47e54a8a |
chore: keep the 48 MB ATP dataset out of the published crate
`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 |
||
|
logaritmiskandClaude Opus 5
|
7de092ba12 |
chore: target releases at the private kellnr registry
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 |
||
|
logaritmiskandClaude Opus 5
|
eeb43e3be1 |
fix: close out four small issues and pin #27's repro
#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 |
||
|
logaritmiskandClaude Opus 5
|
69ddebe21d |
docs: state filtered accessor cost and evidence semantics precisely
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 |
||
|
logaritmisk
|
9c39d1e681 |
test: pin the invariants that make filtered estimates trustworthy
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. |
||
|
logaritmisk
|
50e11cfbfa |
feat: add filtered learning curves
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. |
||
|
logaritmisk
|
d4af048914 |
feat: add filtered_log_evidence
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. |
||
|
logaritmisk
|
bf9d964cae |
refactor!: remove the inert online flag
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. |
||
|
logaritmiskandClaude Opus 5
|
187aede924 |
docs: implementation plan for filtered estimates
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
|
||
|
logaritmiskandClaude Opus 5
|
4fde482e48 |
docs: spec for filtered (forward-only) estimates
`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 |
||
|
logaritmiskandClaude Opus 5
|
9e8515b7cd |
docs: refresh README and CLAUDE.md; add ingest benchmark
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 |
||
|
logaritmiskandClaude Opus 5
|
9506fed4b3 |
chore: add CI, crate metadata, and crate-level documentation
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 |
||
|
logaritmiskandClaude Opus 5
|
6030dc78de |
refactor: unify convergence defaults, validate builders, clear dead code
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
|
||
|
logaritmiskandClaude Opus 5
|
355cdb7e05 |
perf(gaussian): drop the sqrt round-trip from variance-space operations
`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
|
||
|
logaritmiskandClaude Opus 5
|
06b6a68499 |
fix(rayon): remove the aliasing unsafe from the parallel sweep
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
|
||
|
logaritmiskandClaude Opus 5
|
c088214fed |
fix(history): stop reprocessing the slice that was just appended to
`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
|
||
|
logaritmiskandClaude Opus 5
|
0f1a1b8911 |
fix(evidence): accumulate in log space and floor the per-link value
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 |
||
|
logaritmiskandClaude Opus 5
|
0d32690fcc |
fix(quality): support any number of rating groups
`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
|
||
|
logaritmiskandClaude Opus 5
|
6b8bd786d7 |
style: make NaN rejection explicit in score_sigma validation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DnsaJg74eNSva3PJjK2eej |
||
|
logaritmiskandClaude Opus 5
|
f4e2922d59 |
fix: reject ties without draw probability; never report NaN as converged
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 |