The Schedule trait is public API that the engine never uses — wire it into run_chain or make it private #42

New Issue

2026-09-07T14:02:28Z

logaritmisk commented

2026-09-07 14:02:28 +00:00

Found while auditing what the merged T0–T2 PR (#1) deferred and never tracked. Damped/Residual schedules were on that list; chasing them surfaced a present-tense defect in the surface they were meant to extend.

The gap

Schedule, EpsilonOrMax and ScheduleReport are all public — via graph, and ScheduleReport at the crate root (src/lib.rs:152). The trait's entire purpose is to drive factor propagation. The engine does not use it.

=== Schedule::run call sites outside schedule.rs ===
src/game.rs:578:        schedule.run(factors, vars)     <- inside #[doc(hidden)] Game::custom

=== EpsilonOrMax outside schedule.rs ===
src/graph.rs:19:    schedule::{EpsilonOrMax, Schedule, ScheduleReport},   <- the re-export itself

Game::run_chain — the loop that every Game::ranked / Game::scored and all of History actually executes — is hand-rolled. It carries its own while tuple_gt(step, epsilon) && iter < max_iter and calls DiffFactor::propagate directly, never touching a Schedule.

So the only path to the trait is Game::custom, which is #[doc(hidden)] and therefore not documented, not stable, and not listed in the docs. EpsilonOrMax is never constructed anywhere in the crate. A downstream implementor of Schedule has nowhere to plug it in.

This is a public extension point that extends nothing — the same family as Observer::on_batch_processed never firing (#21), ConvergenceReport::slices_skipped always zero (#33), and ConvergenceFailed / NegativePrecision never constructed (#20). Those are all now fixed; this one is still standing.

Two related facts that shape the decision

Damped already shipped, just not as a Schedule. It landed as ConvergenceOptions.alpha, applied inside the hand-rolled loop (src/game.rs, run_chain). The spec's "Damped { eps, max, alpha } schedule" deferral is satisfied in substance. Both May 2026 specs then deferred "wire Schedule into run_chain so Damped becomes a real Schedule", and that never happened.

Residual is not worth building. The spec projects "faster convergence on uneven graphs" (design doc §schedules). Measured, by instrumenting run_chain's iteration counter directly:

even   n=2:  iters=1      uneven n=2:  iters=1
even   n=3:  iters=5      uneven n=3:  iters=5
even   n=5:  iters=5      uneven n=5:  iters=8
even   n=8:  iters=5      uneven n=8:  iters=8
even   n=12: iters=6      uneven n=12: iters=8
even   n=20: iters=6      uneven n=20: iters=8

"Uneven" here is a deliberately hostile field — means spread over 8σ steps with sigmas cycling 1..19, ranked in reverse. Against a max_iter of 30, the loop converges in 1–8 iterations and is flat in team count. A priority-queue schedule could reclaim at most 2–3 iterations of a sub-microsecond loop, while adding allocation and ordering to the hot path. It would plausibly be slower.

There is direct precedent for not taking the spec's performance projections on faith: T3 targeted ≥2× from parallelism and #2's own benchmarks measured 1.0× on realistic workloads. That is the same mistake this would repeat.

So the extension point has no known consumer: Damped exists by another route, Residual is not worth having, and Game::custom is hidden.

The decision

Wire it or remove it. Extending it is the one option the evidence does not support.

Option A — make it private (recommended). Drop Schedule, EpsilonOrMax and ScheduleReport from the public surface; keep them pub(crate) if Game::custom stays. Smallest change, and it makes the public API describe what the crate actually does. Costs the extension point — which nothing is using, and which the crate cannot currently honour anyway.

Option B — wire run_chain through Schedule. Makes the trait real, and Damped could then become a genuine Schedule rather than a bare alpha. This is what the May specs intended.

The risk in B is concrete: run_chain is the hot loop with bit-exactness obligations — tests/determinism.rs asserts bit-identical posteriors across thread counts, and the numerical goldens are cross-validated against the Python/Julia reference. EpsilonOrMax's own doc claims it "Matches the existing Game::likelihoods loop bit-for-bit when given the same factor layout", but nothing tests that claim, because nothing runs both paths on the same input. That claim would need to be proven before it could be relied on.

Whichever way it goes, Game::custom should stop being #[doc(hidden)] — either promoted to real documented API, or removed.

Acceptance

For A:

Schedule, EpsilonOrMax, ScheduleReport are no longer reachable from outside the crate.
Game::custom is either removed or documented.
CHANGELOG.md records the removal.

For B:

run_chain drives propagation through a Schedule.
A test runs the same factor layout through run_chain and through EpsilonOrMax::run and asserts bit-identical results, proving the doc claim that is currently unverified.
tests/determinism.rs and every numerical golden pass unchanged.
A benchmark shows no regression on benches/batch.rs.

Priority

Not urgent — nothing is broken for callers, because nothing can reach it. But it is breaking either way, so it wants the same major bump as the other breaking item (Outcome::winner, #20). 0.4.0 has just shipped; this is 0.5.0 work.

Found while auditing what the merged T0–T2 PR (#1) deferred and never tracked. `Damped`/`Residual` schedules were on that list; chasing them surfaced a present-tense defect in the surface they were meant to extend. ## The gap `Schedule`, `EpsilonOrMax` and `ScheduleReport` are all public — via `graph`, and `ScheduleReport` at the crate root (`src/lib.rs:152`). The trait's entire purpose is to drive factor propagation. **The engine does not use it.** ``` === Schedule::run call sites outside schedule.rs === src/game.rs:578: schedule.run(factors, vars) <- inside #[doc(hidden)] Game::custom === EpsilonOrMax outside schedule.rs === src/graph.rs:19: schedule::{EpsilonOrMax, Schedule, ScheduleReport}, <- the re-export itself ``` `Game::run_chain` — the loop that every `Game::ranked` / `Game::scored` and all of `History` actually executes — is hand-rolled. It carries its own `while tuple_gt(step, epsilon) && iter < max_iter` and calls `DiffFactor::propagate` directly, never touching a `Schedule`. So the only path to the trait is `Game::custom`, which is `#[doc(hidden)]` and therefore not documented, not stable, and not listed in the docs. `EpsilonOrMax` is never constructed anywhere in the crate. A downstream implementor of `Schedule` has nowhere to plug it in. This is a public extension point that extends nothing — the same family as `Observer::on_batch_processed` never firing (#21), `ConvergenceReport::slices_skipped` always zero (#33), and `ConvergenceFailed` / `NegativePrecision` never constructed (#20). Those are all now fixed; this one is still standing. ## Two related facts that shape the decision **`Damped` already shipped, just not as a `Schedule`.** It landed as `ConvergenceOptions.alpha`, applied inside the hand-rolled loop (`src/game.rs`, `run_chain`). The spec's "`Damped { eps, max, alpha }` schedule" deferral is satisfied in substance. Both May 2026 specs then deferred "wire `Schedule` into `run_chain` so `Damped` becomes a real `Schedule`", and that never happened. **`Residual` is not worth building.** The spec projects "faster convergence on uneven graphs" (design doc §schedules). Measured, by instrumenting `run_chain`'s iteration counter directly: ``` even n=2: iters=1 uneven n=2: iters=1 even n=3: iters=5 uneven n=3: iters=5 even n=5: iters=5 uneven n=5: iters=8 even n=8: iters=5 uneven n=8: iters=8 even n=12: iters=6 uneven n=12: iters=8 even n=20: iters=6 uneven n=20: iters=8 ``` "Uneven" here is a deliberately hostile field — means spread over 8σ steps with sigmas cycling 1..19, ranked in reverse. Against a `max_iter` of 30, the loop converges in **1–8 iterations and is flat in team count**. A priority-queue schedule could reclaim at most 2–3 iterations of a sub-microsecond loop, while adding allocation and ordering to the hot path. It would plausibly be slower. There is direct precedent for not taking the spec's performance projections on faith: T3 targeted ≥2× from parallelism and #2's own benchmarks measured **1.0×** on realistic workloads. That is the same mistake this would repeat. So the extension point has **no known consumer**: `Damped` exists by another route, `Residual` is not worth having, and `Game::custom` is hidden. ## The decision Wire it or remove it. Extending it is the one option the evidence does not support. **Option A — make it private (recommended).** Drop `Schedule`, `EpsilonOrMax` and `ScheduleReport` from the public surface; keep them `pub(crate)` if `Game::custom` stays. Smallest change, and it makes the public API describe what the crate actually does. Costs the extension point — which nothing is using, and which the crate cannot currently honour anyway. **Option B — wire `run_chain` through `Schedule`.** Makes the trait real, and `Damped` could then become a genuine `Schedule` rather than a bare `alpha`. This is what the May specs intended. The risk in B is concrete: `run_chain` is the hot loop with bit-exactness obligations — `tests/determinism.rs` asserts bit-identical posteriors across thread counts, and the numerical goldens are cross-validated against the Python/Julia reference. `EpsilonOrMax`'s own doc claims it "Matches the existing `Game::likelihoods` loop bit-for-bit when given the same factor layout", but **nothing tests that claim**, because nothing runs both paths on the same input. That claim would need to be proven before it could be relied on. Whichever way it goes, `Game::custom` should stop being `#[doc(hidden)]` — either promoted to real documented API, or removed. ## Acceptance For A: - `Schedule`, `EpsilonOrMax`, `ScheduleReport` are no longer reachable from outside the crate. - `Game::custom` is either removed or documented. - `CHANGELOG.md` records the removal. For B: - `run_chain` drives propagation through a `Schedule`. - A test runs the same factor layout through `run_chain` and through `EpsilonOrMax::run` and asserts bit-identical results, proving the doc claim that is currently unverified. - `tests/determinism.rs` and every numerical golden pass unchanged. - A benchmark shows no regression on `benches/batch.rs`. ## Priority Not urgent — nothing is broken for callers, because nothing can reach it. But it is **breaking either way**, so it wants the same major bump as the other `breaking` item (`Outcome::winner`, #20). `0.4.0` has just shipped; this is `0.5.0` work.