5d9501307e6c62f3ef4be737782bb1eae062e09e
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  logaritmiskandClaude Opus 5
|
061c481aad |
refactor!: typed discriminators for InferenceError
Six of fifteen variants carried a `&'static str` discriminator, about
thirty magic strings between them, and the only thing a caller could do
with one was print it. Four new enums replace them:
Parameter 13 variants, replacing 9 strings in InvalidParameter
Shape 4 variants, replacing 10 in MismatchedShape
OutcomeKind 2 variants, replacing WrongOutcomeKind's three fields
CompetitorField 2 variants, replacing ConflictingCompetitorConfig's
`InvalidProbability` folds into `InvalidParameter` as
`Parameter::PDraw`. It was a bespoke variant for one scalar while every
other scalar shared `InvalidParameter`, and it omitted the parameter
name — so the same parameter had two mechanisms.
`JointUnavailable { reason: &'static str }` splits into `EmptyHistory`,
`JointRequiresScoredEvents` and `NotPositiveDefinite`. The three are
conditions a caller branches on differently — add events, use
`predict_win_probabilities`, or reconsider the priors — and telling them
apart used to mean string-matching English. One test already proved the
distinction was load-bearing: the blanket conversion mapped the
empty-history case onto the ranked one and `an_empty_history_has_no_joint`
caught it immediately.
`NonFiniteResult` splits into `NonFiniteStep { context, step }` and
`NonFiniteSkill { mu, sigma }`. One `step: (f64, f64)` field was
carrying a sweep step from `converge` and a skill's own moments from a
prediction — two situations in one variant, and a field name that could
only be right for one of them.
`InvalidParameter { name: "beta with point-mass skills" }` becomes
`NoPerformanceVariance`. It was never a parameter out of range: both
values are individually valid and it is their combination that leaves
nothing varying.
Three `Display` impls did not meet the standard the others set, and the
typed data is what makes fixing them possible:
before drift variance is invalid: NaN
after drift variance must be finite and non-negative (got NaN)
before kinds: expected length 3, got 2
after the outcome describes a different number of teams than the
event has: expected 3, got 2
before Game::ranked: expected Outcome::Ranked, got Outcome::Scored
after expected Outcome::Ranked, got Outcome::Scored; call
Game::scored for a scored outcome
`Parameter::range()` states each parameter's actual bounds, which no
`&'static str` name could have. `error::message_tests` renders every one
and asserts each is a sentence rather than a label, and that the three
above now carry a range or a next step.
The four internal `MismatchedShape` kinds — `results`, `times`, `kinds`,
and the weights array — collapse to `Shape::Internal`, whose `Display`
says plainly that reaching it is a bug in this crate. They are checks on
`add_events_with_prior`'s own parallel arrays and are unreachable
through the public API; they stay checked rather than becoming
`debug_assert!`s, because release is where this crate's defects hide.
Closes #74.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
|
||
|
logaritmiskandClaude Opus 5
|
b553c630f5 |
refactor!: K comes first in History, HistoryBuilder and Joint
`K` is the one type parameter people change, and it was last. Naming a
history in a struct field meant writing all four to say one thing:
struct Ladder { history: History<i64, ConstantDrift, NullObserver, String> }
struct Analysis<'h> { joint: Joint<'h, i64, ConstantDrift, NullObserver, &'static str> }
Now:
struct Ladder { history: History<String> }
struct Analysis<'h> { joint: Joint<'h> }
`History<K, T, D, O>`, all four defaulted. Bounds may reference later
parameters, so `D: Drift<T> = ConstantDrift` is legal in third position.
`Joint` gains the same defaults, so `Joint<'h, String>` spells it.
72 call sites swapped, and the reorder makes most of them shorter: 18
now read `History<String>` and the `&'static str` ones read `History`.
The two turbofished builders shrink from
`HistoryBuilder::<Untimed, _, _, String>::new()` to
`HistoryBuilder::<String, Untimed>::new()`.
`Joint` keeps `O` structurally, defaulted rather than removed. #72 notes
it never touches the observer, which is true — but it borrows the whole
`&'h History<K, T, D, O>` and calls `History::resolve_terms`, so dropping
the parameter means either a view type or moving that method off
`History`. The default already buys the entire user-visible benefit,
which was the spelling.
Refs #72.
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
|
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
|
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
|