Gaussian::from_ms overflows for tiny sigma, and three constructors still absorb a negative sign silently #61

Closed
opened 2026-09-09 14:52:28 +00:00 by logaritmisk · 0 comments
Owner

0.8.0 closed this defect class at HistoryBuilder::mu/sigma/beta and at both History and Game ingestion. It is still open one layer down, in the constructors those paths call.

1. from_ms has two overflow regimes, not one

src/gaussian.rs:34-35: pi = 1.0 / (sigma * sigma), tau = mu * pi. The sigma == 0.0 guard is on sigma, but the overflow happens in the square. Measured, mu = 25:

sigma sigma^2 pi tau mu()
1e-150 1e-300 1.0e300 2.5e301 25 — ok
2.4e-154 5.8e-308 (subnormal) 1.7e307 — fine inf inf
1e-160 0 (underflow) inf inf NaN

tau overflows at a larger sigma than pi does, and the threshold depends on mu — so there is a window where pi looks perfectly healthy and only tau is broken. The other direction fails too: from_ms(0.0, 1e160).sigma() is inf, turning an informative prior improper.

A user-visible consequence: Gaussian::from_ms(0.0, 1e-160) == itself is false. Two identical competitor declarations therefore raise ConflictingCompetitorConfig, which is a genuinely confusing error to receive.

Reachable through ingestion, silently:

Member::with_prior(Gaussian::from_ms(1.0, 1e-155))
  -> ingest Ok, converge() = Err, current_skill = (NaN, NaN)
Outcome::scores_with_sigma([3.0, 1.0], 1e-154)
  -> ingest Ok, converge() = Err, current_skill = (NaN, 4.30)

src/factor/margin.rs:84 already documents this exact hazard — "squaring overflows above ~1.3e154 and flushes to zero below ~1.5e-154, and Gaussian's constructors are public so a caller can reach both" — and defends itself with hypot. But the constructor those callers go through is undefended. The same squaring appears at gaussian.rs:245 (Mul<f64>: N(1,1) * 1e-160 -> pi = inf) and rating.rs:87 (beta.powi(2) -> 0 below 1e-162, inf above 1.34e154).

A guarded reciprocal — compute (1/sigma) * (1/sigma) only when 1/(sigma*sigma) comes out zero or non-finite — keeps the normal range bit-identical, so no golden moves.

2. The sign is still silently absorbed at three entry points

Exactly the defect fixed for HistoryBuilder::sigma/beta in 0.8.0. Measured, all bit-identical to their positive counterparts:

Gaussian::from_ms(25.0, -8.33)   == from_ms(25.0, +8.33)      bit-identical
Rating::new(_, -4.17, _)         == Rating::new(_, +4.17, _)  bit-identical
ConstantDrift(-0.0833)           == ConstantDrift(+0.0833)    bit-identical

sigma, beta and gamma all enter only as squares, so a negative value behaves as its absolute value and the sign vanishes without comment.

The worst of the set:

Rating::new(prior, f64::NAN, drift)  ->  Game::ranked(..)  ->  Ok(Gaussian { pi: NaN, tau: NaN })

Game::ranked returns Ok carrying a NaN posterior. Unlike the History paths there is no converge step to catch it, so nothing reports it at all.

ConstantDrift is the same story: NaN, inf and 1e200 all give current_skill returning pi: NaN (though converge does catch those).

Fix

  • from_ms / from_mv: guarded reciprocal, plus reject non-finite and negative sigma.
  • Rating::new: validate beta finite and >= 0, matching HistoryBuilder::beta (zero is legitimate and reaches a measurably different fit — see the 0.8.0 test).
  • ConstantDrift: it is a tuple struct with a pub field, so it cannot intercept construction. Either give it a validating new and seal the field, or — probably better, since it also catches custom Drift impls — validate the drift variance where it is consumed at the ingestion chokepoint, rejecting non-finite or negative.

Note on scope

I twice declared this boundary "complete" this session and was wrong both times — first missing Game entirely, then missing the constructors beneath it. The pattern was validating the layer I had just touched and inferring the rest. Worth a single test that enumerates every public constructor and asserts each rejects negative, NaN and infinite parameters, rather than another round of per-site fixes.

Found by a floating-point audit, 2026-09-09. All figures reproduced independently.

0.8.0 closed this defect class at `HistoryBuilder::mu/sigma/beta` and at both `History` and `Game` ingestion. It is still open one layer down, in the constructors those paths call. ## 1. `from_ms` has two overflow regimes, not one `src/gaussian.rs:34-35`: `pi = 1.0 / (sigma * sigma)`, `tau = mu * pi`. The `sigma == 0.0` guard is on **sigma**, but the overflow happens in the **square**. Measured, `mu = 25`: | sigma | sigma^2 | pi | tau | `mu()` | |---|---|---|---|---| | 1e-150 | 1e-300 | 1.0e300 | 2.5e301 | 25 — ok | | **2.4e-154** | 5.8e-308 (subnormal) | 1.7e307 — fine | **inf** | **inf** | | 1e-160 | 0 (underflow) | **inf** | **inf** | **NaN** | `tau` overflows at a **larger** sigma than `pi` does, and the threshold depends on `mu` — so there is a window where `pi` looks perfectly healthy and only `tau` is broken. The other direction fails too: `from_ms(0.0, 1e160).sigma()` is `inf`, turning an informative prior improper. **A user-visible consequence:** `Gaussian::from_ms(0.0, 1e-160) == itself` is **`false`**. Two *identical* competitor declarations therefore raise `ConflictingCompetitorConfig`, which is a genuinely confusing error to receive. Reachable through ingestion, silently: ``` Member::with_prior(Gaussian::from_ms(1.0, 1e-155)) -> ingest Ok, converge() = Err, current_skill = (NaN, NaN) Outcome::scores_with_sigma([3.0, 1.0], 1e-154) -> ingest Ok, converge() = Err, current_skill = (NaN, 4.30) ``` `src/factor/margin.rs:84` already documents this exact hazard — *"squaring overflows above ~1.3e154 and flushes to zero below ~1.5e-154, and `Gaussian`'s constructors are public so a caller can reach both"* — and defends itself with `hypot`. But the constructor those callers go **through** is undefended. The same squaring appears at `gaussian.rs:245` (`Mul<f64>`: `N(1,1) * 1e-160 -> pi = inf`) and `rating.rs:87` (`beta.powi(2)` -> `0` below 1e-162, `inf` above 1.34e154). A guarded reciprocal — compute `(1/sigma) * (1/sigma)` only when `1/(sigma*sigma)` comes out zero or non-finite — keeps the normal range **bit-identical**, so no golden moves. ## 2. The sign is still silently absorbed at three entry points Exactly the defect fixed for `HistoryBuilder::sigma`/`beta` in 0.8.0. Measured, all bit-identical to their positive counterparts: ``` Gaussian::from_ms(25.0, -8.33) == from_ms(25.0, +8.33) bit-identical Rating::new(_, -4.17, _) == Rating::new(_, +4.17, _) bit-identical ConstantDrift(-0.0833) == ConstantDrift(+0.0833) bit-identical ``` `sigma`, `beta` and `gamma` all enter only as squares, so a negative value behaves as its absolute value and the sign vanishes without comment. **The worst of the set:** ``` Rating::new(prior, f64::NAN, drift) -> Game::ranked(..) -> Ok(Gaussian { pi: NaN, tau: NaN }) ``` `Game::ranked` returns **`Ok`** carrying a NaN posterior. Unlike the `History` paths there is no `converge` step to catch it, so nothing reports it at all. `ConstantDrift` is the same story: `NaN`, `inf` and `1e200` all give `current_skill` returning `pi: NaN` (though `converge` does catch those). ## Fix - `from_ms` / `from_mv`: guarded reciprocal, plus reject non-finite and negative sigma. - `Rating::new`: validate `beta` finite and `>= 0`, matching `HistoryBuilder::beta` (zero is legitimate and reaches a measurably different fit — see the 0.8.0 test). - `ConstantDrift`: it is a tuple struct with a `pub` field, so it cannot intercept construction. Either give it a validating `new` and seal the field, or — probably better, since it also catches custom `Drift` impls — validate the **drift variance** where it is consumed at the ingestion chokepoint, rejecting non-finite or negative. ## Note on scope I twice declared this boundary "complete" this session and was wrong both times — first missing `Game` entirely, then missing the constructors beneath it. The pattern was validating the layer I had just touched and inferring the rest. Worth a single test that enumerates **every** public constructor and asserts each rejects negative, NaN and infinite parameters, rather than another round of per-site fixes. Found by a floating-point audit, 2026-09-09. All figures reproduced independently.
logaritmisk added the apibugnumerics labels 2026-09-09 14:54:02 +00:00
Sign in to join this conversation.