fdd1539cab1f3583be6df67f3bba4471f8ea2ed3
95
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  logaritmiskandClaude Opus 5
|
fdd1539cab |
refactor: one word per concept
Three vocabulary collisions, from #75. **"rating" meant three things**, one the opposite of the exported type. `Rating` is documented as static *configuration* — "this returns what it was told", against every other accessor's "what inference inferred". But `quality`'s parameter was `rating_groups: &[&[Gaussian]]` and its prose said "rating groups" four times, where "rating" means a *posterior* — the one thing `Rating` is documented not to be. Two error messages used it that way too. So a reader who learned `Rating = config` passed `Rating` values to `quality`, which takes `Gaussian`; and one who learned "rating = what comes out" was baffled that `h.rating(&k)` is not their skill. "rating" is now reserved for the type. `quality(teams: &[&[Gaussian]])`, and "every rating is finite" became "every posterior is finite". **"agent" was a private fourth name for a competitor** — ~200 identifiers against 236 uses of "competitor", and it leaked into two `pub` signatures on `TimeSlice`. Now that #73 has made those internal this is a pure rename, so the crate has one word for the entity throughout. **"player" survived in one public signature** — `free_for_all(players:)` plus two doc lines. Renamed, along with three internal closure bindings. Doc examples that use "player" as a *key* are left alone: that is a user's data, not the crate's vocabulary. The panic-message expectations in tests/quality.rs moved with the prose, which is the point of asserting on message text — the tests caught the rename rather than papering over it. Not touched: "performance" (always skill widened by beta), "skill", "member" and "team" are each used for exactly one thing already. Refs #75 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
85c4d0d87d |
fix!: correct eight wrong # Errors sections and seal the error variants
Documentation (#78). Every item below was measured against the code rather than read: - `expected_information_gain` and `predict_ranking` had `# Errors` immediately followed by `# Preconditions`, with the error list stranded at the bottom of the latter — rustdoc rendered a BLANK Errors section on both. The heading now sits with its content. - `predict_outcome`, `predict_ranking` and the free `expected_information_gain` all omitted `GridTooCoarse`. - `predict_margin` claimed `JointUnavailable` "if the LATEST slice holds ranked events". Measured with an early ranked slice and a late scored one: it fails. The condition is *any* slice. - `add_events` documented three errors and can return five more; it also claimed a weights `MismatchedShape` that is unreachable through it, since weights arrive one-per-`Member`. That check belongs to `EventBuilder::weights`, and the doc now says so. - `converge` and `converge_partial` both omitted the drift-variance `InvalidParameter`. `History` gains a hand-written `Debug` (#76). Summarising, not exhaustive — a derived one would print every competitor's skill at every slice. It exists because without it a consumer cannot `#[derive(Debug)]` on any struct holding a `History`, which is how both known consumers store one. `#[non_exhaustive]` on all 17 `InferenceError` struct variants and on `Outcome::Scored` (#74). The enum carried the attribute; no variant did, so adding a field to any of them — and downstream construction of any of them — were both in the public contract. This crate added two variants in two days. The options structs are deliberately NOT sealed. `ConvergenceOptions` and `GameOptions` are constructed by struct literal at 65 sites of which only 8 use `..default()`, and specifying all three convergence fields is a natural complete statement rather than a partial one. That is a real trade-off rather than an oversight, and it is left as a decision on #74. Also spells `UnknownKeys::Reject` explicitly at both sites that wildcarded it. `#[non_exhaustive]` on your own enum gives no exhaustiveness safety net if you then match `_`. Sealing the variants pushed ten test sites from constructing errors to `matches!`, which is the better assertion anyway — an `assert_eq!` against a constructed error breaks whenever a field is added, which is the exact fragility the attribute exists to prevent. BREAKING CHANGE: `InferenceError`'s struct variants and `Outcome::Scored` are `#[non_exhaustive]` — downstream patterns need `..` and downstream construction is no longer possible. Refs #78, #76, #74 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
a0c2f78aed |
feat: add the missing trait impls and make #[must_use] consistent
Trait coverage (#76), all additive: History Debug is still absent - see below HistoryBuilder + Debug (it derived Clone but not Debug) Rating + PartialEq (Gaussian had it; Rating is a Gaussian plus three scalars and had none) Event/Team/Member + PartialEq (input value types with no way to compare them, which made round-trip tests awkward) ConvergenceReport + PartialEq `#[must_use]` (#67). The coverage had no rule: `filtered_log_evidence` had it and `log_evidence` did not; `rating` had it and `current_skill` did not; `Rating::with_drift_scale` had it and `Member::with_drift_scale` did not. Now on the types — `EventBuilder`, `HistoryBuilder`, `Prediction`, `Gaussian`, `OwnedGame` — which covers most method returns at once, plus the `History` accessors individually. `EventBuilder` gets a message, because a dropped builder is the worst case in the set: measured, `h.event(1).team(["x"]).team(["y"]).winner(0)` without `.commit()` leaves `time_slices_len() == 0` and every skill `None`, with no warning at all. And `ConvergenceReport`'s `#[must_use]` moves off the TYPE onto `converge_partial`, where its stated reason is true. It read "from `converge_partial` this may describe a fit that stopped at max_iter" but fired on `converge` too — where that is false, since `converge` returns `Err(NotConverged)` in exactly that case. So the crate's own front-page example warned, and every quickstart had to write `let _ =`. Verified from a consumer crate: `h.converge()?;` now compiles clean. Marking the types made eight method-level attributes redundant, which clippy's `double_must_use` caught — that is the type-level marker doing its job, and the eight are removed. Refs #76, #67 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
4472d98b56 |
refactor!: un-export six types that no caller could reach
`TimeSlice`, `EventKind`, `KeyTable`, `CompetitorStore`, `Competitor` and the `storage` module were all public and none was obtainable from a `History` — `time_slices`, `agents` and `keys` are all private or `pub(crate)`. `TimeSlice` was the worst: `new`, `add_events`, `iteration`, `get_composition` and `get_results` were `pub` on a type you could only build standalone and never feed back into anything. Their sole consumer outside `src/` was `benches/batch.rs`, so a benchmark was dictating six public types. It is rewritten against the public API: a single-slice history's `converge` calls exactly the same per-slice sweep, so capping at one iteration measures the same code path. `N01` had zero references in the entire repository, including inside the crate; removed. `N00` and `N_INF` are EP identities (`Add` and `Mul`) and are now `pub(crate)` — a user reaching for `N_INF` as "an unknown competitor's prior" would get an improper distribution whose `mu()` silently reports 0.0. Adds the accessors their absence forced people around, from #70: `competitors()`, `competitor_count()` and `event_count()` (`size` had no accessor at all). Answering "who is best" previously meant materialising every competitor's full smoothed curve to read the last point of each. `KeyTable::keys` now iterates the dense reverse table rather than the forward `HashMap`, so `competitors()` yields insertion order rather than per-process hash order — the same hazard as #62, caught before it could reach a caller building a standings table. Two `CompetitorStore` methods (`is_empty`, `iter_mut`) had no callers anywhere and are gone; four more are now `#[cfg(test)]`, which is what they always were in practice. Worth recording a mistake: I first deleted `get_composition`/`get_results` on the strength of a "never used" warning, and the build broke — the warning came from the plain-lib target, where `#[cfg(test)]` callers in history.rs are not compiled. A dead-code warning from one target is not evidence about the others. BREAKING CHANGE: `TimeSlice`, `EventKind`, `KeyTable`, `CompetitorStore`, `Competitor`, the `storage` module, `N01`, `N00` and `N_INF` are no longer public. Refs #73, #70 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
dc1f4d5847 |
fix!: make the Time generic reachable
`History<T: Time, ..>` has always been generic over the time axis,
`Untimed` has always been exported, and `Drift<T>` is generic specifically
so that "seasonal or calendar-aware drift is expressible without going
through i64". None of it was reachable from a downstream crate.
Every construction route pinned `T = i64`: `History::builder()`,
`History::builder_with_key()`, and the only `Default` impl on
`HistoryBuilder`. Its fields are private and it had no `new`. So all three
escape routes failed to compile, and a consumer with domain timestamps
had to convert to i64 — which is the exact thing the parameter exists to
avoid. One of `History`'s four type parameters was paid for at every
signature and could never be varied.
`Default` is now generic over `T` and `K`, `HistoryBuilder::new()` exists,
and `time_type::<T2>()` / `key_type::<K2>()` join `drift` and `observer`
as type-changing setters:
History::builder().time_type::<Untimed>().build()
History::builder().key_type::<String>().build()
HistoryBuilder::<Season, _, _, String>::new().build()
`key_type` replaces `builder_with_key`, which could not be turbofished —
`K` sat on the impl rather than the function, so callers had to spell
`History::<i64, _, _, String>::builder_with_key()`. 18 call sites across
15 files migrated.
tests/time_axis.rs is the part that matters. NOTHING in the repository
constructed a non-i64 history, which is precisely why this survived, so
the fix is only half done without a test that exercises the generic. It
defines a `Season(u16)` time type and a `SeasonalDrift` that accumulates
between seasons but not within one — the calendar-aware case the trait's
docs cite — and checks the whole path: fit, converge, and read a learning
curve whose times come back as `Season`, not as integers.
Two of the six tests are controls rather than assertions about output.
`Untimed` must ignore drift entirely, since elapsed is always zero, so
gamma 0.0 and gamma 5.0 must agree bit for bit. And a custom `Drift` must
actually widen a gap across seasons, or the test above would pass whether
or not the drift was consulted at all.
The README's ticked "Generalise a time axis" box is now true.
BREAKING CHANGE: `History::builder_with_key()` is removed. Use
`History::builder().key_type::<K>()`.
Closes #68
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
|
||
|
logaritmiskandClaude Opus 5
|
8dff7513f7 |
fix!: seal ConstantDrift's field so gamma can be validated
`gamma` enters only as `gamma * gamma`, so the sign was squared away: measured against the old public-field form, `ConstantDrift(-0.0833)` produced results bit identical to `ConstantDrift(0.0833)`. The sign was neither rejected nor honoured — it vanished. It could not be checked while the field was a public tuple position, because there was nothing to intercept. Validating inside `variance_for_elapsed` would have been worse: it runs in the sweep, so a construction-time mistake would panic mid-inference, and `Gaussian::from_ms` is a worked example of why that is the wrong place — rejecting NaN there turned the NonFiniteResult reporting path into a crash. So `ConstantDrift::new` is the only way in and it checks, with `gamma()` to read the value back. 129 call sites rewritten across src, tests, benches, examples and the README. The dated plan and spec documents under docs/superpowers are left alone: they record what was built at the time, and rewriting them would falsify that. tests/constructor_validation.rs is the more valuable half. This defect class was closed three times in one session and reopened twice, because each fix validated the layer it had just touched and inferred the rest — `HistoryBuilder`, then `Game`'s own entry points, then the constructors beneath both. A per-site fix cannot notice the site nobody thought of, so that file enumerates every public entry point taking a magnitude and asserts each refuses negative and non-finite values. It found an eleventh defect on its first run: `HistoryBuilder::score_sigma` accepted infinity, because `inf > 0.0` is true and the assert only tested positivity. Fixed, and its own `should_panic` message updated to match. `Gaussian::from_ms` is deliberately exempt from the non-finite half, for the reason above: a broken fit produces a NaN sigma legitimately and `converge` must be allowed to report it. The convergence-level drift-variance check stays and is now tested through a custom `Drift` implementation, since `ConstantDrift` can no longer reach it. That check is the only thing standing between a third-party `Drift` and a NaN fit. BREAKING CHANGE: `ConstantDrift`'s field is private. Replace `ConstantDrift(x)` with `ConstantDrift::new(x)`, and `drift().0` with `drift().gamma()`. `HistoryBuilder::score_sigma` now rejects infinity. Closes #65 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
7aa7fb62dd |
fix: make posterior_of reproducible across processes
`ResolvedTerms::unseen` was a `HashMap<String, f64>` and three float reductions iterated it. Addition is not associative and Rust seeds its default hasher per process, so `posterior_of` returned different bits run to run on identical input: measured over 40 processes, two distinct sigma bit patterns, and five distinct values from `expected_variance_reduction` spanning about 7 ULP. A `BTreeMap` fixes it by construction. 40/40 identical after, 24/16 before. The cross-batch conflict scan had the same cause with a different symptom. It returns on the FIRST conflict, so hash order decided WHICH competitor the error blamed — 15 different competitors named across 40 runs on identical input. The error fired every time; only its content was a lottery, which sends a reader after the wrong key. Now scanned in sorted order. Magnitude was 1-7 ULP throughout, so no decision changes. The cost was reproducibility: a golden test over these would flake at a low rate, which is the worst kind of CI failure to diagnose. tests/cross_process_determinism.rs re-executes the test binary and compares bits, because an in-process test CANNOT see this — every sample in one process shares one hasher seed. That is not hypothetical: tests/determinism.rs compares four thread counts inside one process and passed throughout while this was live. Tuning that fixture took a measurement. Coefficients spread over nine decades detected the bug in roughly one run in forty, because the small terms fall below the running total's ULP and are absorbed whatever the order. Comparable magnitudes keep every term able to change the last bits: 5 of 5 attempts detected it, with 3 to 38 of 40 runs differing. Verified non-vacuous by reverting the BTreeMap and watching it fail. Closes #62 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
ab23476aaf |
fix!: validate the constructors below HistoryBuilder
0.8.0 closed the sign-absorption defect at `HistoryBuilder::mu/sigma/beta`
and at both ingestion paths. It was still open one layer down, in the
constructors those paths call. Measured, all bit identical to their
positive counterparts:
Gaussian::from_ms(25.0, -8.33) == from_ms(25.0, +8.33)
Rating::new(_, -4.17, _) == Rating::new(_, +4.17, _)
ConstantDrift(-0.0833) == ConstantDrift(+0.0833)
sigma, beta and gamma enter only as squares, so the sign vanished without
comment. Worst of the set: `Rating::new(_, NaN, _)` reached `Game::ranked`
which returned **Ok** carrying `Gaussian { pi: NaN, tau: NaN }` — no
`converge` on that path to catch it.
`from_ms` and `Rating::new` now reject. `ConstantDrift` cannot: the field
is public and positional, so there is no constructor to intercept, and
sealing it would break every `ConstantDrift(x)` for a case whose resulting
model is perfectly valid. Documented instead. Its non-finite half IS
rejected — `converge` validates the drift variance each competitor
accumulates, which also covers a custom `Drift` impl.
Two things the tests caught that I had wrong:
NaN sigma must PASS `from_ms`. My first version rejected it, and two
existing tests went red immediately: a broken fit legitimately produces a
NaN sigma from `sqrt` of a negative truncated variance, and the design is
to propagate that to `NonFiniteResult`. Rejecting it turned the reporting
path into a panic inside inference. Written as
`sigma >= 0.0 || sigma.is_nan()` so the intent is explicit rather than
hidden in a negated comparison.
Very small sigma is also not rejected, and that is deliberate: `approx`
produces small truncated sigmas legitimately. `pi = 1/sigma^2` leaves
f64's range below ~1.5e-154 and `tau = mu*pi` overflows sooner, at a
threshold that depends on mu — so there is a band where pi is finite and
only tau is not. Both land on the existing point-mass representation.
Documented, including that such a Gaussian is not equal to itself and can
make two identical declarations report as conflicting.
BREAKING CHANGE: `Gaussian::from_ms` panics on a negative sigma, and
`Rating::new` panics unless beta is finite and non-negative. `converge`
returns `InvalidParameter` for a non-finite drift variance.
Closes #61
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
|
||
|
logaritmiskandClaude Opus 5
|
bbc7705c75 |
fix!: report an unresolvable prediction grid instead of clamping
`grid_shape` asked for 12 nodes across the narrowest feature and then clamped to MAX_GRID_POINTS with no detection that the request was not met. Past `step/sigma ~ 1.7` the trapezoid rule stops resolving the density, and the result is unbounded: sigma_a step/sig_a P(a first) exact total 2.0e-3 0.86 0.515953 0.515953 1.000000 1.0e-3 1.72 0.517185 0.515953 1.002388 1.0e-4 17.17 2.791336 0.515953 5.410065 A probability of 2.79. Reachable through `predict_outcome` with a pinned reference competitor — a documented pattern — where `predict_outcome` and `predict_win_probabilities` disagreed 44x and `predict_outcome` was the wrong one. There is no useful answer on the far side of that cliff, so this reports `GridTooCoarse` rather than guessing, and the message points at `predict_win_probabilities`, which answers the same matchup through adaptive quadrature and is accurate there to 1e-13. The floor is 4 nodes per feature rather than the 12 requested, because the request carries margin: measured accurate to 2.2e-12 at 1.4 nodes per sigma and wrong by 1.2e-3 at 0.7. This also fixes the `ln k` ceiling violation. `expected_information_gain` weights `probability * divergence`, so probabilities of 3.97 and 2.62 made it return 3.237828 nats against `ln 2 = 0.693147` — 4.67x over. The crate's docs call that ceiling its sharpest test and record a prototype once returning 4.77 nats; it was live again by a different route. The new sweep then caught a second, independent defect: `kl_divergence` returned NEGATIVE values, worst -5.55e-17, exactly one ULP of its `- 1.0`. Rewritten as `0.5*(u - ln1p(u)) + gap^2/(2*var_p)` with `u = var_q/var_p - 1`, so both terms are non-negative by construction. It is also more accurate where it matters: at `u = 1e-9` the old form returned 0.0 where the true value is 2.5e-19, and well-conditioned cases are unchanged. tests/prediction_bounds.rs sweeps rather than spot-checks, because a single fixture cannot defend a bound like this — the previous check passed throughout. It asserts the sweep still reaches the coarse-grid regime, so it cannot quietly stop testing the case it was written for. BREAKING CHANGE: `predict_outcome`, `predict_ranking` and `expected_information_gain` return `GridTooCoarse` for matchups whose performance sigmas are too far apart to integrate on one grid. They previously returned wrong answers, including probabilities above 1. Closes #55, closes #56 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
83bdb84152 |
fix!: collapse a drift too small to represent, on a relative threshold
`time_expanded_joint` collapsed consecutive appearances only at `drift <= 0.0` exactly. Anything smaller-but-positive got an explicit `1.0 / drift` precision, which the matrix cannot hold: at `drift = 1e-16` the entry is `1e16`, and `1e16 + 0.28` rounds back to `1e16`, so the prior and the event contrasts are annihilated in the stored f64 before the factorisation ever runs. Measured, 8 competitors over 15 slices: drift_scale before after 1e-6 1.3e-3 relative error exact 1e-7..1e-9 Err(JointUnavailable) exact 1e-10 12 200x TOO SMALL, as Ok exact At 1e-10 the caller was handed sigma = 0.0055 where the truth is 0.6108 — a 111x overconfident interval, returned as a success. This is representation, not conditioning. Solved in 200-digit precision the same system converges smoothly onto the collapsed value and is flat from 1e-16 to 1e-40, so the quantity is perfectly well conditioned. That also rules out the obvious fix: symmetric (Jacobi) equilibration measured 30x WORSE, because the information is gone from the assembled matrix before any solver sees it. The fix has to be at assembly. The threshold balances the two errors that trade off. Ignoring a real drift costs about `drift / V`; representing one costs about `EPSILON * V / drift`. They cross at `V * sqrt(EPSILON)`, scaled to each competitor's own prior variance. Ordinary drift is far above it and unaffected — the default gamma accumulates 0.0069 per unit time against a threshold of 1.0e-6 — and the test asserts both halves: everything below the threshold reaches the collapsed answer bit-identically, and a drift of 1e-2 still moves it, so the test cannot pass by collapsing everything. Also corrects the `JointUnavailable` message, which asserted "a competitor has neither a proper prior nor any evidence" for a fixture where every competitor had both. BREAKING CHANGE: a drift variance below `prior_variance * sqrt(EPSILON)` now collapses two appearances into one latent variable. Affected fits previously returned a badly wrong variance or an error. Closes #57 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
8b20e0c560 |
fix!: reject non-finite weights at ingestion
Measured, a NaN weight behaved exactly as `0.0`:
weight NaN -> Ok, converged: true, 1 iteration, step (0.0, 0.0)
skill pi 0.027777777777777776, tau 0.0
weight 0.0 -> Ok, same skill, bit for bit
So a NaN arriving from a division or a parse was indistinguishable from
a deliberate zero, and the fit reported itself as cleanly converged.
Worth correcting an earlier description of this: the event does not
vanish. The member contributes nothing, which is precisely what weight
zero means, and that equivalence is what makes it undetectable rather
than merely wrong.
Zero and negative weights stay accepted. Both are expressible choices
about how much a member contributes, and tests/degenerate_inputs.rs pins
their behaviour deliberately; only values that are not quantities at all
are rejected. A test asserts they still ingest, so the new check cannot
quietly widen.
This completes the boundary: every malformed input that previously
produced a plausible answer — a one-team event, an empty team, a
non-finite score, a non-finite weight — now fails where it enters.
BREAKING CHANGE: an event carrying a non-finite weight returns
`InvalidParameter` instead of silently treating that member as weightless.
Refs #18
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
|
||
|
logaritmiskandClaude Opus 5
|
f692906ce4 |
docs: state what the joint's cost actually scales in
A consumer measured an 8x difference in solve time between two fits over the same events, the same slices and the same ~2,000 nodes: career fit (gamma = 0) 787 ms drifting fit (gamma = 0.15) 6214 ms Entirely the collapse rule. A competitor with zero drift contributes one variable however long the history, so a drift-free fit's joint is smaller than a drifting one's by roughly the slice count — and to factorise, by its cube. Choosing a drift configuration is therefore also choosing a query cost, and nothing said so. Documented on `Joint`, on `Joint::variables` and on `posterior_of`, with the measurement. `variables()` is named as the number that decides affordability, since it can be read before committing to a batch. Also states the thing the consumer proposed as a future optimisation, because it is already true: an absence is not an appearance, so a competitor seen in the first and last of a hundred slices contributes two variables rather than a hundred. The matrix is already as small as the model allows on that axis. tests/joint_handle.rs pins the mechanism — ten slices, two competitors, twenty variables drifting against two at `gamma = 0` — so a change to the collapse rule cannot quietly remove the property the docs now promise. Refs #51 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
e493f47e99 |
feat!: add History::register and History::rating, and reject config conflicts across batches
Three things #38 asked for, on a premise that had half dissolved. The
issue argued from "captured at first appearance", "missing it is silent"
and "missing it is permanent";
|
||
|
logaritmiskandClaude Opus 5
|
4f6360128d |
feat!: validate mu, sigma and beta on HistoryBuilder
The last three unvalidated setters, beside `p_draw`, `score_sigma` and `convergence`, which all assert eagerly. Measured before choosing bounds: beta = 0 -> works, pi 0.0211 (vs 0.0193 at the default) beta = -4.17 -> bit-identical to +4.17 sigma = -8.33 -> bit-identical to +8.33 sigma = 0 -> NonFiniteResult, current_skill returns tau: NaN sigma = inf -> same mu = NaN -> same So the bounds are not the obvious ones. `beta = 0` is legitimate and meaningful — performance is then exactly skill, and the fit moves measurably rather than degenerating — so zero is allowed and a test pins that it reaches a different answer, since "allowed" would otherwise be indistinguishable from "unchecked". The negative cases are the quiet ones. `sigma` and `beta` enter inference only as squares, so a negative value behaves as its absolute value and the sign is dropped without comment. That is the same defect `Member::with_drift_scale` already rejects, for the reason already written there. The non-finite cases are detected today — `converge` reports NonFiniteResult — but a caller who reads `current_skill` first is handed `tau: NaN`, so rejecting at the boundary is what actually closes it. BREAKING CHANGE: `HistoryBuilder::mu`, `sigma` and `beta` now panic on values they previously accepted, matching the existing behaviour of `p_draw`, `score_sigma` and `convergence`. Refs #18 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
eff63dfa2a |
feat!: make a short fit an error and raise the default iteration cap
`ITERATIONS` was 30, and overrunning it returned `Ok` with `converged: false`. Both halves were wrong. The cap is a runaway guard, not a budget: the sweep exits as soon as the step falls below `epsilon`, so a high cap costs nothing on a history that converges. Measured on one needing four sweeps, `max_iter` 30 and 100_000 both finish in 4 iterations and ~130us. So 30 could never make anything faster — it could only stop a healthy history early, and it did: 160 events over 100 competitors already needs 42. Not scaled to the history, because iteration count tracks how loopy the graph is rather than how big it is. At a fixed 320 events over 40 slices, varying only the competitors sharing them: 3 competitors needs 2_789 sweeps, 10 needs 1_068, 100 needs 90, 400 needs 2. Three orders of magnitude on identical event and slice counts, so any formula in those two numbers would be badly wrong on some real shape. A single value set high enough that reaching it means oscillation is the honest version. With the cap raised, stopping at it means something is genuinely wrong, so `converge` now returns `InferenceError::NotConverged` rather than a flag on a success. A short fit is wrong by a little — every rating finite, the ordering sensible, nothing saying the numbers were still moving — and a flag has to be checked while `let _ = h.converge()` is the natural way not to. That is not hypothetical: it is how a real defect hid in this crate's own test suite. `converge_partial` returns the short fit for callers who want one. Only a single existing test needed it, which is the evidence that a capped fit is a deliberate choice rather than the common case. Also corrects the `ITERATIONS` docs, which claimed convergence cost is "roughly linear in the cap". It is linear in the iterations actually run. BREAKING CHANGE: `History::converge` returns `Err(NotConverged)` where it previously returned `Ok` with `converged: false`. Callers that want the old behaviour should use `History::converge_partial`. The default `max_iter` changes from 30 to 10_000, so a history that was silently truncated will now converge properly and its numbers will move. Closes #50 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
8e4d6a637d |
fix: reject malformed events at the ingestion boundary
A one-team event reached `run_chain`, which builds one diff link per
adjacent pair of teams, leaving it to index `links[1..]` on an empty
vector. That panicked with "range start index 1 out of range for slice
of length 0" — from `History::add_events`, in a release build, through
entirely safe API.
An empty team was the quieter half of the same gap. It contributes no
performance, so a malformed event converged and handed back a finite,
plausible-looking posterior for whoever it was matched against. That is
this crate's characteristic defect: a public surface reporting a
constant that looks like an answer.
A non-finite score was the third. `converge` did report NonFiniteResult,
so it was detected — but a caller reading `current_skill` before
converging was handed `tau: NaN` with nothing to say so.
`NotEnoughTeams` and `EmptyTeam` already existed. They were checked on
the prediction paths and nowhere else, which is exactly why ingestion
could still manufacture the states they describe. The checks go in
`add_events_with_prior` alongside the tie check, for the same reason
that one is there: every ingestion route lands on it, so `record_winner`,
`record_draw` and `EventBuilder` inherit them rather than each needing
their own.
Also corrects documentation that had been stating the opposite of the
code since
|
||
|
logaritmiskandClaude Opus 5
|
1bb6bb31d8 |
feat: factorise the joint once with History::joint
`posterior_of`, `posterior_of_at` and `expected_variance_reduction` each
built the joint precision matrix, factorised it, asked one question and
threw it away. The factorisation is O(n^3) in the history's appearances
and depends only on the fit, so a caller asking about every pair in a
standings table, every cell in a grid, or every candidate in an
active-learning sweep paid for the same factorisation once per question.
`History::joint()` returns a `Joint` handle that pays it once. Measured
on 1976 appearances, 90 queries: 68.4s one-shot against 745ms factorise
plus 93ms of queries — 81.6x, with bit-identical answers. Per query,
Criterion at 480 appearances: 9.0ms one-shot against 48us cached, 187x.
The handle borrows the history, which is what makes it correct with no
invalidation logic: the borrow checker forbids adding events or refitting
while it is alive, so there is no window in which the factorisation could
describe a fit that no longer exists. It also makes the lifetime of the
n^2 factor explicit rather than parking it in the history forever — at
4000 appearances that is 128MB, which is not something to cache silently.
Every question the joint answers turns out to be a bilinear form,
c^T A^-1 a = (L^-1 c) . (L^-1 a)
so no caller ever needs L^-1 c itself. Replacing the general solve with a
forward substitution drops the back substitution as wasted work, halving
a query, and removes a failure mode: a variance as `c . (A^-1 c)` is a
difference of products that can round negative, where `|L^-1 c|^2` is a
sum of squares and cannot.
The one-shot calls are unchanged in cost and now delegate to the handle,
so the two paths cannot drift apart. tests/joint_handle.rs asserts they
agree bit for bit, including at pinned times, under UnknownKeys::Prior,
and across candidate matchups.
Refs #51
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
|
||
|
logaritmiskandClaude Opus 5
|
f345e7690e |
fix!: make the joint span slices, not just the latest one
`posterior_of` shipped in 0.5.0 reading a single slice. Measured against a real Through-Time history that answers almost nothing: ustat's round fit is 76 per-day slices whose last one holds a solo round, so 0 of 55 pair differences resolved and the single node that did was degenerate — a one-competitor slice has no correlation to account for and returns the marginal unchanged. That was my mistake, and the fixture chose it. I validated against single-slice histories, which is exactly the shape that cannot reveal the problem. In a library whose premise is skill over time, competitors are read at *their own* last appearance and those are different slices by construction. The joint is now time-expanded: one variable per appearance, linked by the prior on a first appearance, the drift between consecutive ones, and the within-slice event contrasts. Consecutive appearances with no drift between them are the same variable rather than two joined by an infinite precision, which keeps the matrix positive-definite when a competitor is pinned with `drift_scale = 0`. `posterior_of` now reads each competitor at their own latest appearance, which is where `current_skill` reads them, so the two agree about which posterior they describe. Adds `posterior_of_at(time, terms)` for a comparison anchored to a moment, matching `learning_curve`'s reading. Validated against a hand-written exact posterior for a two-competitor, two-slice history — the precision matrix is spelled out in the test rather than obtained from the crate, so it is an independent check rather than a restatement. Also pinned: competitors last seen in different slices now compare at all, means still agree with the marginals, zero drift makes slice layout irrelevant, and more drift widens a comparison across time. BREAKING CHANGE: `posterior_of` and `expected_variance_reduction` now consider the whole history rather than its latest slice, so results change for any multi-slice history. `JointUnavailable` is now returned when *any* slice holds ranked events, not just the last. Refs #46, #47 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
7cf45db5cf |
feat: add expected_variance_reduction for scored active learning
#49: `expected_information_gain` enumerates discrete outcomes, so a consumer recording continuous scores cannot ask which matchup to run next. The issue flagged this as possibly a research question, since "expected variance reduction under EP may not have a clean closed form even for Gaussian likelihoods". It does. Observing a scored event is a rank-one update to the precision matrix, so Sherman-Morrison gives reduction = (c^T L^-1 a)^2 / (v + a^T L^-1 a) for target functional c and matchup contrast a. Verified against an actual refit on four candidate matchups: agreement to 1e-9 relative. Two consequences worth stating. There is no expectation to take. The expression depends on which matchup is played but not on how it turns out, because for a Gaussian likelihood the posterior variance update is data-independent. Pinned by `the_outcome_does_not_change_the_reduction`, which refits with scores of (3, 1), (100, -50) and (0, 0) and gets the same answer. The name keeps the term the active-learning literature uses; no averaging happens. It is also far cheaper than its ranked counterpart — one linear solve rather than a full inference pass per possible outcome — because `c^T L^-1 a` and `a^T L^-1 a` share the same solve. `target` is deliberately the same linear-functional shape as `posterior_of`, as the issue proposed, so the two share a concept rather than inventing two. The load-bearing test is the refit comparison. An acquisition function is the archetype of a surface that returns finite, plausible, monotone numbers while being wrong, and then quietly selects worse matchups forever; ranking behaviour alone would not catch that. Closes #49 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
c866210c65 |
feat: add History::predict_margin for scored matchups
#48: every predict_* answers "who wins", and a consumer recording scores never asks that. It wants the interval on the result, and having none it hand-fitted a noise law whose fitted node weight came out at 0.0 — so the quoted sigma was 5.83 whether the competitor had forty rounds or none, against real residual spreads of 5.8 and 12.44. `predict_margin` composes the three things that make a scored result uncertain: the joint posterior over the competitors, their per-event performance noise, and the observation noise on the score. It widens as the model knows less — measured, sigma 2.48 against an opponent with forty rounds, 3.38 against one seen once, wider still against one never seen — which is the property the hand-fitted law lost. It is a margin, not a score, and that is not a shortcut. Scored ingestion reduces every event to `score_a - score_b` before inference, so the absolute level is discarded: shifting every score in a history by +100 or -1000 produces a bit-identical fit, verified. There is no information from which to predict what a competitor will *score*. Returning one would be a number derived entirely from the prior, which is exactly the plausible constant this crate keeps finding and removing. `posterior_of` now honours `UnknownKeys::Prior`, which gives #48 its second requirement — "I have never seen this competitor, here is the prior-informed answer". An unseen competitor shares no event with the slice, so it is independent by construction and its variance is additive rather than part of the solve. Worth recording for expectations: for a *margin* the joint buys little over adding marginals (2.4798 against 2.5112 here), because a margin is a difference and differences are where the loopy underestimate and the ignored correlation cancel. The gain here is having a predictive distribution at all. `posterior_of`'s correlation handling earns its keep on sums and single nodes instead — see tests/additive_model.rs. Closes #48 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
c52e2550af |
feat: add History::posterior_of for a linear combination of competitors
#46: every accessor returns a per-competitor marginal, and almost nothing a consumer publishes is one competitor. Combining marginals assumes independence, and competitors are correlated through every event they share. `posterior_of(&[(a, 1.0), (b, -1.0)])` returns the posterior of that combination with the correlation intact. Validated against the exact linear-Gaussian posterior on both a tree and a loopy fixture, for differences and for single competitors: agreement to 1e-9 relative in every case. The investigation that preceded this is why it is not a covariance accessor. Marginals from loopy message passing are about half the true width, and ignoring correlation overstates a difference — the two errors partially cancel, leaving 1.327x rather than 2.646x. Bolting true correlations onto the existing marginals would have given 0.765 against a true 1.524, which is overconfident: the direction the reporter specifically called unsafe. Rebuilding the joint from the factor structure fixes both at once, and a single-competitor query now returns the exact marginal rather than the narrow one. The precision matrix depends only on structure — who played whom, with what weights and what noise — not on the observed outcomes, and the means were already exact. So only the second moment is reconstructed. Known limits, all deliberate and documented on the method: - Latest slice only. A functional spanning times, such as "current versus career", needs the time-expanded joint and is not covered. - Scored events only. A ranked outcome's truncation is EP-approximated and its converged factors are not retained after inference, so ranked slices return `JointUnavailable` rather than a plausible wrong number. - Dense Cholesky, O(n^3) per query in the slice's competitor count: 38.8us at 50, 5.66ms at 400, 49.1ms at 800. Fine for the sizes this serves today; caching the factorization per slice would make repeat queries O(n^2), and sparsity is the next step after that. Refs #46, #47, #48 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
71554fd944 |
feat: add UnknownKeys::Prior, and explain why there is no Skip
#44's third ask was an opt-in mode so a caller with partially-known teams need not pre-filter. The requested shape was `Skip` — drop unknown members. Measured, that is the wrong mode to build. A team's performance is the *sum* of its members, so dropping one drops its variance too. On a two-member team with one unknown: SKIP (drop the member) : performance sigma 2.37 PRIOR (member at prior) : performance sigma 6.53 (2.76x wider) Skipping makes the model *more* certain because it knows *less*, which is backwards. `Prior` is also the answer the model already gives for a competitor it knows about but has no evidence for — measured, such a competitor sits at sigma 4.99 against the prior's 6.0 — so it corresponds to a state the model can actually be in. Skipping does not. So the enum is `Reject` (default, unchanged) and `Prior`, and it is `#[non_exhaustive]` in case a real use for skipping turns up later. Placed on `HistoryBuilder` rather than per-call. Neither consumer wants it to vary between queries: one scores thousands of candidate matchups in a loop, the other's headline feature is predicting a competitor nobody has faced. That makes it a property of how the model is being used, and keeps five prediction signatures unchanged. This also gives #48 the semantics it asked for — "I have never seen this competitor, here is the prior-informed answer" — which it needs for predicting a course nobody has played. `an_unknown_member_widens_its_team_rather_than_narrowing_it` pins the property that ruled `Skip` out, so a future convenience cannot quietly reintroduce it. Closes #44. Refs #48 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
2cf21a753d |
docs: record that the event log is the source of truth, and why
#45 asked whether a fitted `History` can be persisted, and noted that the absence of `serde` "reads as an omission rather than a decision, which invites exactly this issue from every consumer in turn". Fair. Documents the decision on `History` along with the reasoning that makes it one: `converge` reaches a fixed point determined by the events, ratings and configuration alone, so a snapshot would carry no information the event log does not — it would cache the computation, never the answer. It also bounds what a snapshot could buy, since that is the question a consumer actually has. Re-converging an unchanged history costs one iteration, measured at 0.91 ms against 365 ms cold on 2 000 events, so it would make a cold restart cheap and do nothing for appends. Appending one event moves its participants more than a sigma across their whole history, so that re-convergence is real work rather than repeated work. Closes #45 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
c12bc830a5 |
feat!: name the unknown key, expose tail probabilities, flag short fits
Three issues from two downstream consumers, all small, all sharing a theme: the crate had the information and would not hand it over. #44 — `UnknownKey { team: 0, member: 0 }` did not say which key. A consumer upgrading 0.1.2 -> 0.4.1 had every one of 5591 predictions return this error, fell back to a neutral 0.5, and lost its entire metadata model for a day. Nothing crashed and nothing logged; it was found by sweeping an unrelated parameter and noticing the output did not move. The 0.4.0 change that made unknown keys an error was right — the error was just too anonymous to act on. It now carries the key's `Debug` rendering, and its `Display` says what to do about it. The precondition is documented on every prediction entry point, which the reporter said would alone have saved the day. #43 — `cdf` was `pub(crate)`, so a consumer asking "is this competitor below the cutoff" approximated it with a `mu + z * sigma` band and had no way to say what confidence any `z` bought. Adds `Gaussian::probability_below` / `probability_above`. The second is separate on purpose: `1 - cdf` collapses to exactly zero past ~8.3 sigma, and a stopping rule is evaluated precisely there. Both route through the survival function added in 0.4.1, so this is visibility rather than new numerics. #50 — `ConvergenceReport` was not `#[must_use]`, so the one signal that a fit stopped short was trivially discarded. It now is, and that immediately found 78 sites doing exactly that — including this crate's own ATP example, which was capped at 10 sweeps when the history needs 30. The example now reads the report and says so. `ITERATIONS = 30` is documented as the floor it is, with the three measurements to hand: 400 events over 100 competitors already stops there at ~7e-3 against a 1e-6 tolerance, the ATP example needs 30 at a much looser one, and a consumer's 2000-node model needs 76 to 161. BREAKING CHANGE: `InferenceError::UnknownKey` gains a `key` field, and the prediction methods now require `K: Debug` in order to fill it. Closes #43, #50. Refs #44 — its third ask, an opt-in `UnknownKeys::Skip` mode, is a live API question and deliberately not answered here. 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 |
||
|
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
|
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
|
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 |
||
|
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
|
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 |
||
|
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
|
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
|
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
|
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
|
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 |