logaritmisk/trueskill-tt
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Activity

Input validation is debug-only, and the History ingestion path bypasses InferenceError entirely #18

New Issue
Closed
opened 2026-08-04 19:18:55 +00:00 by logaritmisk · 4 comments
logaritmisk commented 2026-08-04 19:18:55 +00:00
Owner
Copy Link
Copy Source

The crate has a well-formed InferenceError type and Result-returning public constructors, but the checks that actually protect the engine are debug_assert!s — they vanish in release, which is how #8 (NaN on draws) reaches users silently.

Where the checks are debug-only

Check Location
result.len() == teams.len() src/game.rs:190
weights dimensions match teams src/game.rs:194
p_draw in [0, 1) src/game.rs:201
no ties when p_draw == 0.0 src/game.rs:205
alpha in (0, 1] src/game.rs:213, 252
scores.len() == teams.len() src/game.rs:240
score_sigma > 0.0 src/game.rs:251, src/factor/margin.rs:24
resolved score_sigma > 0.0 src/history.rs:737
sigma > 0.0 src/outcome.rs:62 (Outcome::scores_with_sigma)
weights length matches team src/event_builder.rs:61

In release, violating any of these produces silent garbage (NaN, pi = inf) or an out-of-bounds panic deep inside run_chain rather than a clean error at the boundary.

The structural problem

Game::ranked and Game::scored (src/game.rs:441, 480) do validate properly and return InferenceError. But they immediately delegate to ranked_with_arena/scored_with_arena, and the History path calls those directly — TimeSlice::iteration (src/time_slice.rs:328, 336), Event::iteration_direct (src/time_slice.rs:143), TimeSlice::log_evidence (src/time_slice.rs:533, 543). So every event ingested through History::add_events, record_winner, record_draw, or event(…).commit() reaches the engine having been checked only for:

  • outcome team-count vs teams length (src/history.rs:701)
  • the top-level shape checks in add_events_with_prior (src/history.rs:472-499)

Everything else in the table above — the tie/p_draw interaction, p_draw range, alpha range, score_sigma positivity — is unchecked on the path most callers actually use.

HistoryBuilder is inconsistent about it too: score_sigma uses a hard assert! that panics (src/history.rs:83), while p_draw (src/history.rs:72) and convergence (src/history.rs:91) accept anything, including a negative p_draw, p_draw >= 1.0, or alpha = 0.0 (which makes EP never update and silently never converge).

Fix

Validate once, at the ingestion boundary, returning InferenceError:

  • Move the debug_assert! conditions in ranked_with_arena/scored_with_arena into a shared validation function that both the public Game constructors and the History ingestion path call.
  • Have History::add_events reject the tie/p_draw == 0 combination (see #8 for the semantics decision), out-of-range p_draw, non-positive score_sigma, and out-of-range alpha.
  • Make HistoryBuilder consistent: either all setters validate eagerly and panic, or all defer to a build() -> Result<…>. Mixing assert! in one setter with no check in its neighbours is the worst of both.
  • Keep debug_assert!s inside the engine as invariant documentation, but stop relying on them as the only guard.

Acceptance

  • Every condition in the table above produces an InferenceError from the public API in a release build.
  • Tests for each, run in release (cargo test --release), not just debug.
  • HistoryBuilder setter validation is consistent and documented.
The crate has a well-formed `InferenceError` type and `Result`-returning public constructors, but the checks that actually protect the engine are `debug_assert!`s — they vanish in release, which is how #8 (NaN on draws) reaches users silently. ## Where the checks are debug-only | Check | Location | |---|---| | `result.len() == teams.len()` | `src/game.rs:190` | | weights dimensions match teams | `src/game.rs:194` | | `p_draw` in `[0, 1)` | `src/game.rs:201` | | **no ties when `p_draw == 0.0`** | `src/game.rs:205` | | `alpha` in `(0, 1]` | `src/game.rs:213`, `252` | | `scores.len() == teams.len()` | `src/game.rs:240` | | `score_sigma > 0.0` | `src/game.rs:251`, `src/factor/margin.rs:24` | | resolved `score_sigma > 0.0` | `src/history.rs:737` | | `sigma > 0.0` | `src/outcome.rs:62` (`Outcome::scores_with_sigma`) | | weights length matches team | `src/event_builder.rs:61` | In release, violating any of these produces silent garbage (NaN, `pi = inf`) or an out-of-bounds panic deep inside `run_chain` rather than a clean error at the boundary. ## The structural problem `Game::ranked` and `Game::scored` (`src/game.rs:441`, `480`) do validate properly and return `InferenceError`. But they immediately delegate to `ranked_with_arena`/`scored_with_arena`, and **the `History` path calls those directly** — `TimeSlice::iteration` (`src/time_slice.rs:328`, `336`), `Event::iteration_direct` (`src/time_slice.rs:143`), `TimeSlice::log_evidence` (`src/time_slice.rs:533`, `543`). So every event ingested through `History::add_events`, `record_winner`, `record_draw`, or `event(…).commit()` reaches the engine having been checked only for: - outcome team-count vs teams length (`src/history.rs:701`) - the top-level shape checks in `add_events_with_prior` (`src/history.rs:472-499`) Everything else in the table above — the tie/`p_draw` interaction, `p_draw` range, `alpha` range, `score_sigma` positivity — is unchecked on the path most callers actually use. `HistoryBuilder` is inconsistent about it too: `score_sigma` uses a hard `assert!` that panics (`src/history.rs:83`), while `p_draw` (`src/history.rs:72`) and `convergence` (`src/history.rs:91`) accept anything, including a negative `p_draw`, `p_draw >= 1.0`, or `alpha = 0.0` (which makes EP never update and silently never converge). ## Fix Validate once, at the ingestion boundary, returning `InferenceError`: - Move the `debug_assert!` conditions in `ranked_with_arena`/`scored_with_arena` into a shared validation function that both the public `Game` constructors and the `History` ingestion path call. - Have `History::add_events` reject the tie/`p_draw == 0` combination (see #8 for the semantics decision), out-of-range `p_draw`, non-positive `score_sigma`, and out-of-range `alpha`. - Make `HistoryBuilder` consistent: either all setters validate eagerly and panic, or all defer to a `build() -> Result<…>`. Mixing `assert!` in one setter with no check in its neighbours is the worst of both. - Keep `debug_assert!`s inside the engine as invariant documentation, but stop relying on them as the only guard. ## Acceptance - Every condition in the table above produces an `InferenceError` from the public API in a release build. - Tests for each, run in release (`cargo test --release`), not just debug. - `HistoryBuilder` setter validation is consistent and documented.
logaritmisk commented 2026-08-04 20:12:45 +00:00
Author
Owner
Copy Link
Copy Source

Partly done — staying open.

Now enforced in release, returning InferenceError from the public API:

  • ties with p_draw == 0.0 → TieWithoutDrawProbability, checked in add_events_with_prior (the chokepoint every route reaches, including record_draw) and in Game::ranked
  • resolved score_sigma → InvalidParameter, at ingestion. Outcome::scores_with_sigma no longer debug_assert!s, so construction is infallible and the value is validated where it is used
  • p_draw range and alpha range → eager assert! in HistoryBuilder, matching the convention score_sigma already used there. Builder validation is now consistent across all four setters

Still debug_assert!-only, so still unchecked in release on the History path:

  • result.len() == teams.len() (src/game.rs:190)
  • weights dimensions match teams (src/game.rs:194, src/event_builder.rs:61)
  • scores.len() == teams.len() (src/game.rs:240)
  • p_draw and alpha ranges inside ranked_with_arena / scored_with_arena

The shared validation function the issue proposes is still the right shape: ranked_with_arena and scored_with_arena return Self rather than Result, so promoting their asserts means threading Result up through TimeSlice::iteration, Event::compute and log_evidence. That is a mechanical but wide change, and worth doing in one deliberate pass rather than piecemeal.

Note the shape checks are partly covered in practice: History::add_events validates outcome-vs-teams length up front, so the internal mismatches are reachable mainly through the Game constructors directly.

Partly done — **staying open.** Now enforced in release, returning `InferenceError` from the public API: - ties with `p_draw == 0.0` → `TieWithoutDrawProbability`, checked in `add_events_with_prior` (the chokepoint every route reaches, including `record_draw`) and in `Game::ranked` - resolved `score_sigma` → `InvalidParameter`, at ingestion. `Outcome::scores_with_sigma` no longer `debug_assert!`s, so construction is infallible and the value is validated where it is used - `p_draw` range and `alpha` range → eager `assert!` in `HistoryBuilder`, matching the convention `score_sigma` already used there. Builder validation is now consistent across all four setters **Still `debug_assert!`-only**, so still unchecked in release on the `History` path: - `result.len() == teams.len()` (`src/game.rs:190`) - weights dimensions match teams (`src/game.rs:194`, `src/event_builder.rs:61`) - `scores.len() == teams.len()` (`src/game.rs:240`) - `p_draw` and `alpha` ranges inside `ranked_with_arena` / `scored_with_arena` The shared validation function the issue proposes is still the right shape: `ranked_with_arena` and `scored_with_arena` return `Self` rather than `Result`, so promoting their asserts means threading `Result` up through `TimeSlice::iteration`, `Event::compute` and `log_evidence`. That is a mechanical but wide change, and worth doing in one deliberate pass rather than piecemeal. Note the shape checks are partly covered in practice: `History::add_events` validates outcome-vs-teams length up front, so the internal mismatches are reachable mainly through the `Game` constructors directly.
logaritmisk commented 2026-08-27 15:54:01 +00:00
Author
Owner
Copy Link
Copy Source

One more item enforced in release — 1ac3b21 — but staying open, and the remainder needs a decision from you rather than more work from me.

Done: EventBuilder::weights (src/event_builder.rs:61). It guarded the length match with debug_assert!, so release accepted a mismatch, silently dropped the weights, and ingested the event anyway. The setters return Self to keep the chain fluent so they cannot return a Result; the builder now records the first failure and commit returns it as MismatchedShape. The weights are not applied on mismatch either, so a partially-weighted team cannot reach the history by another route.

Two tests in tests/degenerate_inputs.rs, whose CI job runs in release — the only place the old behaviour differed. Worth noting the second one needed strengthening before it meant anything: as first written it committed a one-team event, which ingestion rejects for an unrelated reason, so it passed under a mutation that disabled the whole check. With two teams the assertion is load-bearing, and disabling the error path now fails both.

The remainder is a public-API decision. Promoting the asserts in ranked_with_arena / scored_with_arena means threading Result up through every caller, and the production call graph is:

ranked_with_arena / scored_with_arena
  -> Event::compute
  -> TimeSlice::iteration (both branches)
  -> TimeSlice::log_evidence
       -> iteration_direct, sweep_color_groups, iterate_to_convergence
       -> new_forward_info, new_backward_info, filtered_step
            -> History::iteration, filtered_pass

That surfaces as:

  • log_evidence() -> Result<f64>
  • log_evidence_for() -> Result<f64>
  • filtered_log_evidence() -> Result<f64>
  • filtered_learning_curve() -> Result<Vec<...>>
  • filtered_learning_curves() -> Result<HashMap<...>>

Every query method becomes fallible, to guard an invariant that add_events already establishes at ingestion — your own note above says as much: "the shape checks are partly covered in practice… the internal mismatches are reachable mainly through the Game constructors directly." And the Game constructors already validate independently and return Result.

So the real question is which of these you want, and I don't think it's mine to pick:

  1. Thread it through. Full release enforcement, at the cost of Result on every query method.
  2. Validate at the boundary instead. Add release-checked shape validation in TimeSlice::add_events (narrow — it is called from two places), then keep the game-level debug_assert!s and document that they are guarded by it. Enforcement in release without touching the query API.
  3. Leave them. Document that ranked_with_arena/scored_with_arena are pub(crate) and reached only with pre-validated data, so the asserts are internal-consistency checks rather than input validation.

I lean toward 2 — it gets the enforcement this issue asks for, at the boundary where the data actually enters, without making log_evidence() fallible. But that is a judgment about the API you'll live with. Say which and I'll implement it.

One more item enforced in release — `1ac3b21` — but **staying open**, and the remainder needs a decision from you rather than more work from me. **Done: `EventBuilder::weights` (`src/event_builder.rs:61`).** It guarded the length match with `debug_assert!`, so release accepted a mismatch, silently dropped the weights, and ingested the event anyway. The setters return `Self` to keep the chain fluent so they cannot return a `Result`; the builder now records the first failure and `commit` returns it as `MismatchedShape`. The weights are not applied on mismatch either, so a partially-weighted team cannot reach the history by another route. Two tests in `tests/degenerate_inputs.rs`, whose CI job runs in release — the only place the old behaviour differed. Worth noting the second one needed strengthening before it meant anything: as first written it committed a *one-team* event, which ingestion rejects for an unrelated reason, so it passed under a mutation that disabled the whole check. With two teams the assertion is load-bearing, and disabling the error path now fails both. --- **The remainder is a public-API decision.** Promoting the asserts in `ranked_with_arena` / `scored_with_arena` means threading `Result` up through every caller, and the production call graph is: ``` ranked_with_arena / scored_with_arena -> Event::compute -> TimeSlice::iteration (both branches) -> TimeSlice::log_evidence -> iteration_direct, sweep_color_groups, iterate_to_convergence -> new_forward_info, new_backward_info, filtered_step -> History::iteration, filtered_pass ``` That surfaces as: - `log_evidence() -> Result<f64>` - `log_evidence_for() -> Result<f64>` - `filtered_log_evidence() -> Result<f64>` - `filtered_learning_curve() -> Result<Vec<...>>` - `filtered_learning_curves() -> Result<HashMap<...>>` Every query method becomes fallible, to guard an invariant that `add_events` already establishes at ingestion — your own note above says as much: *"the shape checks are partly covered in practice… the internal mismatches are reachable mainly through the `Game` constructors directly."* And the `Game` constructors already validate independently and return `Result`. So the real question is **which of these you want**, and I don't think it's mine to pick: 1. **Thread it through.** Full release enforcement, at the cost of `Result` on every query method. 2. **Validate at the boundary instead.** Add release-checked shape validation in `TimeSlice::add_events` (narrow — it is called from two places), then keep the game-level `debug_assert!`s and document that they are guarded by it. Enforcement in release without touching the query API. 3. **Leave them.** Document that `ranked_with_arena`/`scored_with_arena` are `pub(crate)` and reached only with pre-validated data, so the asserts are internal-consistency checks rather than input validation. I lean toward 2 — it gets the enforcement this issue asks for, at the boundary where the data actually enters, without making `log_evidence()` fallible. But that is a judgment about the API you'll live with. Say which and I'll implement it.
logaritmisk added the apibug labels 2026-09-07 13:52:03 +00:00
logaritmisk commented 2026-09-08 13:54:18 +00:00
Author
Owner
Copy Link
Copy Source

Re-checked against main and mostly closed out. 7c6965c.

Your acceptance table

Every row now either returns an InferenceError in release or is unreachable with bad data through the public API. p_draw, ties at p_draw == 0, alpha, score_sigma, Outcome::scores_with_sigma, EventBuilder::weights and the HistoryBuilder inconsistency all landed earlier. The three remaining debug_assert!s in src/game.rs (ranks, weights dims, scores length) are still debug_assert!, but the boundary above them validates first, so bad data cannot reach them.

What was still live, and it was the thing you predicted

You wrote that the symptom was "an out-of-bounds panic deep inside run_chain rather than a clean error at the boundary". That was still exactly true, and reproducible from safe API in a release build:

History::add_events(one team) -> panicked at src/game.rs:317
                                 "range start index 1 out of range for slice of length 0"

run_chain builds one diff link per adjacent pair of teams, so a one-team event left it indexing links[1..] on an empty vector. NotEnoughTeams already existed — checked on the prediction paths and nowhere else, which is why ingestion could still produce the state it describes.

Two more from the same gap:

  • An empty team was accepted. It contributes no performance, so the event converged and returned a finite posterior for its opponent — pi 0.0730, tau -0.3447 from a default prior. That is this crate's recurring defect rather than a new one: a public surface reporting a constant that looks like an answer.
  • A non-finite score was accepted. converge did report NonFiniteResult, so it was detected — but a caller reading current_skill first got tau: NaN handed back with nothing to say so.

All three now fail at the boundary, and the checks sit in add_events_with_prior beside the tie check for the reason that one is there: every route lands on it, so record_winner, record_draw and EventBuilder inherit them rather than each needing their own. tests/ingestion_shape.rs covers them in release mode.

While there: the comment in tests/degenerate_inputs.rs claiming a one-team event was "rejected for an unrelated reason" was wrong when it was written. It panicked. Corrected.

Left open deliberately

  • NaN weight is accepted and silently no-ops — the event has no effect and converged: true is reported. It is a sibling of the zero/negative-weight behaviour that tests/degenerate_inputs.rs deliberately pins as observed behaviour, so it wants the same policy call rather than a unilateral fix.
  • HistoryBuilder::mu/sigma/beta are unvalidated. sigma(-1.0) and beta(-1.0) build and converge to plausible garbage. Your consistency demand named p_draw/score_sigma/convergence, which are done; these three are the remaining inconsistency in the same builder.
  • The structural question in your second comment — infallible ranked_with_arena/scored_with_arena, TimeSlice::add_events returning (), infallible query methods — is untouched. None of options 1/2/3 was chosen. That is still a decision rather than work.

Happy to close this and refile those three as their own issues if you would rather not carry a mostly-done one; say which.

Re-checked against main and mostly closed out. `7c6965c`. ## Your acceptance table Every row now either returns an `InferenceError` in release or is unreachable with bad data through the public API. `p_draw`, ties at `p_draw == 0`, `alpha`, `score_sigma`, `Outcome::scores_with_sigma`, `EventBuilder::weights` and the `HistoryBuilder` inconsistency all landed earlier. The three remaining `debug_assert!`s in `src/game.rs` (ranks, weights dims, scores length) are still `debug_assert!`, but the boundary above them validates first, so bad data cannot reach them. ## What was still live, and it was the thing you predicted You wrote that the symptom was "an out-of-bounds panic deep inside `run_chain` rather than a clean error at the boundary". That was still exactly true, and reproducible from safe API in a release build: ``` History::add_events(one team) -> panicked at src/game.rs:317 "range start index 1 out of range for slice of length 0" ``` `run_chain` builds one diff link per adjacent pair of teams, so a one-team event left it indexing `links[1..]` on an empty vector. `NotEnoughTeams` already existed — checked on the prediction paths and nowhere else, which is why ingestion could still produce the state it describes. Two more from the same gap: - **An empty team was accepted.** It contributes no performance, so the event converged and returned a finite posterior for its opponent — `pi 0.0730, tau -0.3447` from a default prior. That is this crate's recurring defect rather than a new one: a public surface reporting a constant that looks like an answer. - **A non-finite score was accepted.** `converge` did report `NonFiniteResult`, so it was detected — but a caller reading `current_skill` first got `tau: NaN` handed back with nothing to say so. All three now fail at the boundary, and the checks sit in `add_events_with_prior` beside the tie check for the reason that one is there: every route lands on it, so `record_winner`, `record_draw` and `EventBuilder` inherit them rather than each needing their own. `tests/ingestion_shape.rs` covers them in release mode. While there: the comment in `tests/degenerate_inputs.rs` claiming a one-team event was "rejected for an unrelated reason" was wrong when it was written. It panicked. Corrected. ## Left open deliberately - **NaN weight** is accepted and silently no-ops — the event has no effect and `converged: true` is reported. It is a sibling of the zero/negative-weight behaviour that `tests/degenerate_inputs.rs` deliberately pins as observed behaviour, so it wants the same policy call rather than a unilateral fix. - **`HistoryBuilder::mu`/`sigma`/`beta` are unvalidated.** `sigma(-1.0)` and `beta(-1.0)` build and converge to plausible garbage. Your consistency demand named `p_draw`/`score_sigma`/`convergence`, which are done; these three are the remaining inconsistency in the same builder. - **The structural question in your second comment** — infallible `ranked_with_arena`/`scored_with_arena`, `TimeSlice::add_events` returning `()`, infallible query methods — is untouched. None of options 1/2/3 was chosen. That is still a decision rather than work. Happy to close this and refile those three as their own issues if you would rather not carry a mostly-done one; say which.
logaritmisk commented 2026-09-08 19:15:01 +00:00
Author
Owner
Copy Link
Copy Source

Closing. Everything in the acceptance table is done, and so is the panic — but a correction first, because I got this wrong once already in this thread.

I said the boundary was complete, and it was not

My earlier comment reported the panic fixed. It was fixed at History's ingestion chokepoint, which every History route lands on, and I generalised from that to "the boundary is closed". Game is a separate public entry point that does not pass through that chokepoint, and all four defects were still live there:

Game::ranked(&[&[a]], ..)  -> PANIC at src/game.rs:317
Game::scored(&[&[a]], ..)  -> PANIC at src/game.rs:317
Game::ranked(&[&[], &[a]]) -> Ok, finite posterior for the opponent
Game::scored(.., [NaN, 1]) -> Ok

Same panic, same line, from safe API in release. I only found it because I went looking before closing this rather than after. Fixed in a73afa5, with Game::validate_teams shared by both constructors and tests asserting the well-formed constructors still succeed.

That mistake has a shape worth naming, because it is the second time in this repo: fix a thing on one path, validate on that path, then report the property as holding generally. It is the same error as the latest-slice joint.

The full list, now closed
one-team event panic → NotEnoughTeams, on both entry points
empty team Ok with a plausible posterior → EmptyTeam
non-finite score Ok → InvalidParameter
non-finite weight Ok, behaving as 0.0 bit for bit → InvalidParameter
mu / sigma / beta accepted → panic, matching p_draw / score_sigma / convergence

The weight one is worth a note. It did not produce garbage — it produced exactly what weight 0.0 produces, pi 0.027777777777777776, tau 0.0, while converge reported converged: true after one iteration with step (0.0, 0.0). A NaN from a division was indistinguishable from a deliberate zero. Zero and negative weights stay accepted, with a test asserting so, since those are expressible choices that tests/degenerate_inputs.rs pins on purpose.

sigma and beta were the same class: negative values returned bit-identical results to their positives, because both enter only as squares. The sign vanished silently. beta = 0 is allowed and reaches a measurably different fit — a test pins that, since "allowed" would otherwise be indistinguishable from "unchecked".

The structural question

Your second comment escalated to the shape of the internals: infallible ranked_with_arena / scored_with_arena, TimeSlice::add_events returning (), infallible query methods.

That has changed under it. Both are pub(crate) — they are not public API, so "the ingestion path bypasses InferenceError" no longer describes them, and nothing outside the crate can hand them bad data now that both public entry points validate. log_evidence() -> f64 remains infallible, but converge returning NotConverged and NonFiniteResult means a caller cannot hold a broken fit without having been told.

So I am not refiling it. If you want those signatures changed it is a shape preference worth its own issue with its own argument, not a live defect.

Not refiled either: nothing else remains. #54 carries the benchmark gating from #26.

Closing. Everything in the acceptance table is done, and so is the panic — but a correction first, because I got this wrong once already in this thread. ## I said the boundary was complete, and it was not My earlier comment reported the panic fixed. It was fixed at `History`'s ingestion chokepoint, which every `History` route lands on, and I generalised from that to "the boundary is closed". `Game` is a *separate* public entry point that does not pass through that chokepoint, and all four defects were still live there: ``` Game::ranked(&[&[a]], ..) -> PANIC at src/game.rs:317 Game::scored(&[&[a]], ..) -> PANIC at src/game.rs:317 Game::ranked(&[&[], &[a]]) -> Ok, finite posterior for the opponent Game::scored(.., [NaN, 1]) -> Ok ``` Same panic, same line, from safe API in release. I only found it because I went looking before closing this rather than after. Fixed in `a73afa5`, with `Game::validate_teams` shared by both constructors and tests asserting the well-formed constructors still succeed. That mistake has a shape worth naming, because it is the second time in this repo: fix a thing on one path, validate on that path, then report the property as holding generally. It is the same error as the latest-slice joint. ## The full list, now closed | | | |---|---| | one-team event | panic → `NotEnoughTeams`, on both entry points | | empty team | `Ok` with a plausible posterior → `EmptyTeam` | | non-finite score | `Ok` → `InvalidParameter` | | non-finite weight | `Ok`, behaving as `0.0` bit for bit → `InvalidParameter` | | `mu` / `sigma` / `beta` | accepted → panic, matching `p_draw` / `score_sigma` / `convergence` | The weight one is worth a note. It did not produce garbage — it produced **exactly** what weight `0.0` produces, `pi 0.027777777777777776, tau 0.0`, while `converge` reported `converged: true` after one iteration with step `(0.0, 0.0)`. A NaN from a division was indistinguishable from a deliberate zero. Zero and negative weights stay accepted, with a test asserting so, since those are expressible choices that `tests/degenerate_inputs.rs` pins on purpose. `sigma` and `beta` were the same class: negative values returned **bit-identical** results to their positives, because both enter only as squares. The sign vanished silently. `beta = 0` is allowed and reaches a measurably different fit — a test pins that, since "allowed" would otherwise be indistinguishable from "unchecked". ## The structural question Your second comment escalated to the shape of the internals: infallible `ranked_with_arena` / `scored_with_arena`, `TimeSlice::add_events` returning `()`, infallible query methods. That has changed under it. Both are `pub(crate)` — they are not public API, so "the ingestion path bypasses `InferenceError`" no longer describes them, and nothing outside the crate can hand them bad data now that both public entry points validate. `log_evidence() -> f64` remains infallible, but `converge` returning `NotConverged` and `NonFiniteResult` means a caller cannot hold a broken fit without having been told. So I am not refiling it. If you want those signatures changed it is a shape preference worth its own issue with its own argument, not a live defect. Not refiled either: nothing else remains. #54 carries the benchmark gating from #26.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
api
Shape or ergonomics of the public surface
 blocked
Waiting on another issue or an external decision
 breaking
Resolving it changes the public API; wants a major bump
 bug
Behaviour is wrong, including silently wrong
 decision
Needs a call to be made, not code to be written
 docs
Documentation, including doc comments and the README
 enhancement
New capability the crate does not have
 infra
CI, release process, packaging
 numerics
Floating-point accuracy, cancellation, convergence maths
 performance
Speed or memory
 refactor
Internal cleanup with no intended behaviour change
 testing
Test coverage and test infrastructure
No labels api bug
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
logaritmisk
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: logaritmisk/trueskill-tt#18