Gaussian's operators are EP internals wearing arithmetic's clothes, and / can return a confident wrong number #71

New Issue

2026-09-09T17:54:58Z

logaritmisk commented

2026-09-09 17:54:58 +00:00

Gaussian publicly implements Mul, Div, Add, Sub. They are the EP product, cavity and variance-space convolutions — not arithmetic. Measured:

a = N(mu 10, sigma 2)   b = N(mu 4, sigma 3)   c = N(mu 1, sigma 1)

a * b                 mu=    8.1538  sigma= 1.6641   <- not 40
a - b                 mu=    6.0000  sigma= 3.6056   <- sigma GREW, 2 -> sqrt(4+9)
a * N(1,0)            mu=       NaN  sigma= 0.0000   <- "multiply by one"
a / c                 mu=    0.0000  sigma=    inf   <- pi = -0.75

a*(b+c).mu = 8.5714   vs   (a*b)+(a*c).mu = 10.9538  <- Mul does not distribute over Add

The `/` case is this crate's signature defect, on a public operator

Div is the EP cavity and can legitimately produce a negative precision — pi = -0.75 above. That is not a distribution. But mu() and sigma() guard pi <= 0.0 and report 0.0 and inf, so:

let gap = a / c;
println!("{}", gap.mu());   // prints a confident 0

No panic, no Debug marker, no is_proper(). A caller who reads / as division gets a plausible constant — the exact failure class the crate has spent this session eliminating everywhere else.

a * Gaussian::from_ms(1.0, 0.0) — "scale by one", to anyone reading * as arithmetic — returns mu = NaN.

The doc comments on the impls are one line each and written for a maintainer ("Factor product: nat-param add. Hot path"). All four render on the public rustdoc page with no warning.

Fix, in preference order

Make them pub(crate) inherent methods with honest names — ep_product, cavity, convolve, convolve_diff. Nothing in a user's workflow needs operator syntax; inference does. The trap disappears and no power is lost. This is the "easy to use, powerful" answer.
If they stay public: lead each doc with the warning, and add Gaussian::is_proper(&self) -> bool so the negative-precision result is detectable at all.
Also pub(crate) for pi() and tau(). They are the representation. Storing natural parameters is a performance decision — "message passing becomes add/subtract" — and should not be a public contract.

The user-facing set is good and should stay: from_ms, mu, sigma, probability_below, probability_above. Two currently pub(crate) members deserve promoting: from_mv (construct from mean and variance without the sqrt round trip) and variance().

N00 and N_INF are exported, undocumented, and are EP identities — N00 for Add, N_INF for Mul. A user reaching for N_INF as "an unknown competitor's prior" gets an improper distribution whose mu() silently reports 0.0. If the operators go pub(crate), these have no public meaning at all. N01 has zero references anywhere in the repository, including inside the crate.

Breaks: any downstream a * b; pi()/tau() callers; removes N01/N00/N_INF from the public API.

Found by an API audit, 2026-09-09; every line of the output above reproduced independently.

`Gaussian` publicly implements `Mul`, `Div`, `Add`, `Sub`. They are the EP product, cavity and variance-space convolutions — not arithmetic. Measured: ``` a = N(mu 10, sigma 2) b = N(mu 4, sigma 3) c = N(mu 1, sigma 1) a * b mu= 8.1538 sigma= 1.6641 <- not 40 a - b mu= 6.0000 sigma= 3.6056 <- sigma GREW, 2 -> sqrt(4+9) a * N(1,0) mu= NaN sigma= 0.0000 <- "multiply by one" a / c mu= 0.0000 sigma= inf <- pi = -0.75 a*(b+c).mu = 8.5714 vs (a*b)+(a*c).mu = 10.9538 <- Mul does not distribute over Add ``` ## The `/` case is this crate's signature defect, on a public operator `Div` is the EP cavity and can legitimately produce a **negative precision** — `pi = -0.75` above. That is not a distribution. But `mu()` and `sigma()` guard `pi <= 0.0` and report `0.0` and `inf`, so: ```rust let gap = a / c; println!("{}", gap.mu()); // prints a confident 0 ``` No panic, no `Debug` marker, no `is_proper()`. A caller who reads `/` as division gets a plausible constant — the exact failure class the crate has spent this session eliminating everywhere else. `a * Gaussian::from_ms(1.0, 0.0)` — "scale by one", to anyone reading `*` as arithmetic — returns `mu = NaN`. The doc comments on the impls are one line each and written for a maintainer (*"Factor product: nat-param add. Hot path"*). All four render on the public rustdoc page with no warning. ## Fix, in preference order 1. **Make them `pub(crate)` inherent methods with honest names** — `ep_product`, `cavity`, `convolve`, `convolve_diff`. Nothing in a user's workflow needs operator syntax; inference does. The trap disappears and no power is lost. This is the "easy to use, powerful" answer. 2. If they stay public: lead each doc with the warning, and add `Gaussian::is_proper(&self) -> bool` so the negative-precision result is detectable at all. 3. **Also `pub(crate)` for `pi()` and `tau()`.** They are the representation. Storing natural parameters is a performance decision — "message passing becomes add/subtract" — and should not be a public contract. The user-facing set is good and should stay: `from_ms`, `mu`, `sigma`, `probability_below`, `probability_above`. Two currently `pub(crate)` members deserve **promoting**: `from_mv` (construct from mean and variance without the sqrt round trip) and `variance()`. ## Related `N00` and `N_INF` are exported, undocumented, and are EP identities — `N00` for `Add`, `N_INF` for `Mul`. A user reaching for `N_INF` as "an unknown competitor's prior" gets an improper distribution whose `mu()` silently reports `0.0`. If the operators go `pub(crate)`, these have no public meaning at all. `N01` has **zero references anywhere in the repository**, including inside the crate. **Breaks:** any downstream `a * b`; `pi()`/`tau()` callers; removes `N01`/`N00`/`N_INF` from the public API. Found by an API audit, 2026-09-09; every line of the output above reproduced independently.

logaritmisk referenced this issue

2026-09-09 17:55:48 +00:00

Six exported types are unreachable from `History` — removing them shrinks the API for free #73

logaritmisk added the api breaking bug labels 2026-09-09 17:58:08 +00:00

logaritmisk referenced this issue from a commit

2026-09-09 21:13:14 +00:00

feat!: Gaussian's EP operations stop wearing arithmetic's clothes

logaritmisk closed this issue

2026-09-09 21:13:14 +00:00

logaritmisk referenced this issue from a commit

2026-09-09 21:13:14 +00:00

Merge api/gaussian-operators (#71)

logaritmisk commented

2026-09-09 21:13:37 +00:00

Option 1, in full. 076a7de (merged as ddbac87).

The four trait impls are gone. In their place, pub(crate) inherent methods that say what they do — ep_product, cavity, convolve, convolve_diff — plus scale for the one operation that genuinely was arithmetic (the old Mul<f64>, used for per-member weights). pi() and tau() are pub(crate) too.

The public surface is now exactly the good user-facing set, plus the two promotions:

from_ms  from_mv  mu  sigma  variance  probability_below  probability_above

Three things worth recording from doing it:

The in-crate call sites read better afterwards, not worse. raw * lf.msg() became raw.ep_product(lf.msg()), and (m - performance.exclude(..)) * (1.0 / w) became m.convolve_diff(performance.exclude(..)).scale(1.0 / w). The old spelling made a variance-space convolution and an EP product look like the same kind of thing.

Four integration tests asserted bit-identity on (pi, tau) — event_builder_members, joint_handle, registration, validation — and integration tests are external crates, so they lost access. They assert on (mu, variance) now: still assert_eq!, still exact, and since 1/pi and tau/pi are deterministic, bit-equal natural parameters give bit-equal moments. a_nan_sigma_passes_through_from_ms dropped its || g.pi().is_nan() half, because sigma() substitutes only for pi <= 0 and pi == inf — NaN reaches it only from a NaN precision, so the sigma() check alone is exact.

benches/gaussian.rs is deleted. It timed two f64 additions through the public operators and nothing else. Keeping four public trait impls alive to feed it is precisely what #73 objected to when a benchmark was dictating five public types; the same paths run under batch and history_converge through the real call chain.

N00/N_INF were already pub(crate) and N01 already deleted, so the "Related" section needed nothing.

Option 1, in full. 076a7de (merged as ddbac87). The four trait impls are gone. In their place, `pub(crate)` inherent methods that say what they do — `ep_product`, `cavity`, `convolve`, `convolve_diff` — plus `scale` for the one operation that genuinely was arithmetic (the old `Mul<f64>`, used for per-member weights). `pi()` and `tau()` are `pub(crate)` too. The public surface is now exactly the good user-facing set, plus the two promotions: ``` from_ms from_mv mu sigma variance probability_below probability_above ``` Three things worth recording from doing it: **The in-crate call sites read better afterwards, not worse.** `raw * lf.msg()` became `raw.ep_product(lf.msg())`, and `(m - performance.exclude(..)) * (1.0 / w)` became `m.convolve_diff(performance.exclude(..)).scale(1.0 / w)`. The old spelling made a variance-space convolution and an EP product look like the same kind of thing. **Four integration tests asserted bit-identity on `(pi, tau)`** — `event_builder_members`, `joint_handle`, `registration`, `validation` — and integration tests are external crates, so they lost access. They assert on `(mu, variance)` now: still `assert_eq!`, still exact, and since `1/pi` and `tau/pi` are deterministic, bit-equal natural parameters give bit-equal moments. `a_nan_sigma_passes_through_from_ms` dropped its `|| g.pi().is_nan()` half, because `sigma()` substitutes only for `pi <= 0` and `pi == inf` — NaN reaches it only from a NaN precision, so the `sigma()` check alone is exact. **`benches/gaussian.rs` is deleted.** It timed two f64 additions through the public operators and nothing else. Keeping four public trait impls alive to feed it is precisely what #73 objected to when a benchmark was dictating five public types; the same paths run under `batch` and `history_converge` through the real call chain. `N00`/`N_INF` were already `pub(crate)` and `N01` already deleted, so the "Related" section needed nothing.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: logaritmisk/trueskill-tt#71

Gaussian's operators are EP internals wearing arithmetic's clothes, and / can return a confident wrong number #71

The / case is this crate's signature defect, on a public operator

Fix, in preference order

Related

`Gaussian`'s operators are EP internals wearing arithmetic's clothes, and `/` can return a confident wrong number #71

The `/` case is this crate's signature defect, on a public operator