Opt-in synchronous-EP Schedule with snapshot + barrier merge #6

New Issue

2026-08-04T18:54:37Z

logaritmisk commented

2026-08-04 18:54:37 +00:00

Deferred from #2 (T3). Spec: docs/superpowers/specs/2026-04-23-trueskill-engine-redesign-design.md §6, "Within-slice color-group parallelism" (the Alternative paragraph).

Why

T3 implemented the asynchronous-EP strategy: color-group partitioning, events in the same color run concurrently, colors run sequentially. This preserves today's semantics exactly but only parallelizes as well as the coloring allows — and on realistic workloads the coloring doesn't give rayon enough to chew on (see #4).

Synchronous EP is the alternative the spec names: all events read from a frozen skill snapshot, write deltas to thread-local buffers, and a barrier merges them. It is trivially parallel — no coloring, no unsafe disjoint-index writes — at the cost of weaker per-iteration convergence, which needs damping to compensate.

Current state

src/schedule.rs has exactly one Schedule impl:

EpsilonOrMax (src/schedule.rs:28) — forward/backward sweeps until step ≤ eps or iter == max.

The damping half of the prerequisite already exists, though built game-local rather than as a Schedule: ConvergenceOptions::alpha, Gaussian::damp_natural, and propagate_with_alpha on both TruncFactor and MarginFactor.

Note the Schedule trait signature is currently factor-level:

fn run(&self, factors: &mut [BuiltinFactor], vars: &mut VarStore) -> ScheduleReport;

Spec §5 describes a generic Schedule::run<W: Workload>(&self, workload: &mut W) shared across all three convergence loops. A slice-level synchronous schedule needs that generalization, or a separate seam at the TimeSlice layer — decide which as part of this work.

Scope

Snapshot-read / thread-local-delta-write / barrier-merge sweep at the TimeSlice level.
Opt-in only. Asynchronous EP stays the default; this must not change results for anyone who doesn't ask for it.
Reuse the existing alpha damping rather than inventing a second damping path.

Acceptance

Converges to the same fixed point as the async path (within test epsilons) on the existing history fixtures.
Deterministic across thread counts, same bar as tests/determinism.rs: bit-identical mu/sigma via f64::to_bits() at RAYON_NUM_THREADS={1,2,4,8}.
Benchmarked against the async path on all three benches/history_converge.rs workloads — including iteration count, not just wall-clock, since weaker per-iteration convergence can eat the parallel win.

Deferred from #2 (T3). Spec: `docs/superpowers/specs/2026-04-23-trueskill-engine-redesign-design.md` §6, "Within-slice color-group parallelism" (the *Alternative* paragraph). ## Why T3 implemented the asynchronous-EP strategy: color-group partitioning, events in the same color run concurrently, colors run sequentially. This preserves today's semantics exactly but only parallelizes as well as the coloring allows — and on realistic workloads the coloring doesn't give rayon enough to chew on (see #4). Synchronous EP is the alternative the spec names: all events read from a frozen skill snapshot, write deltas to thread-local buffers, and a barrier merges them. It is trivially parallel — no coloring, no `unsafe` disjoint-index writes — at the cost of weaker per-iteration convergence, which needs damping to compensate. ## Current state `src/schedule.rs` has exactly one `Schedule` impl: - `EpsilonOrMax` (`src/schedule.rs:28`) — forward/backward sweeps until step ≤ eps or iter == max. The damping half of the prerequisite already exists, though built game-local rather than as a `Schedule`: `ConvergenceOptions::alpha`, `Gaussian::damp_natural`, and `propagate_with_alpha` on both `TruncFactor` and `MarginFactor`. Note the `Schedule` trait signature is currently factor-level: ```rust fn run(&self, factors: &mut [BuiltinFactor], vars: &mut VarStore) -> ScheduleReport; ``` Spec §5 describes a generic `Schedule::run<W: Workload>(&self, workload: &mut W)` shared across all three convergence loops. A slice-level synchronous schedule needs that generalization, or a separate seam at the `TimeSlice` layer — decide which as part of this work. ## Scope - Snapshot-read / thread-local-delta-write / barrier-merge sweep at the `TimeSlice` level. - Opt-in only. Asynchronous EP stays the default; this must not change results for anyone who doesn't ask for it. - Reuse the existing `alpha` damping rather than inventing a second damping path. ## Acceptance - Converges to the same fixed point as the async path (within test epsilons) on the existing history fixtures. - Deterministic across thread counts, same bar as `tests/determinism.rs`: bit-identical `mu`/`sigma` via `f64::to_bits()` at `RAYON_NUM_THREADS={1,2,4,8}`. - Benchmarked against the async path on all three `benches/history_converge.rs` workloads — including iteration *count*, not just wall-clock, since weaker per-iteration convergence can eat the parallel win. ## Related - #4 — cross-slice parallelism, the other deferred parallelism path. If #4 delivers, this may not be worth the complexity; measure before committing.

logaritmisk referenced this issue

2026-08-04 19:20:15 +00:00

Convergence configuration is inconsistent: two different iteration defaults, and `converged: false` for trivially-converged graphs #22

logaritmisk added the enhancement numerics labels 2026-09-07 13:52:02 +00:00

logaritmisk commented

2026-09-08 00:15:01 +00:00

Heads-up: this issue's premise no longer holds after 633a503.

It proposes a synchronous-EP Schedule with snapshot and barrier merge. The Schedule trait has been removed, along with EpsilonOrMax, ScheduleReport, Game::custom, and the Factor / BuiltinFactor / RankDiffFactor / TeamSumFactor machinery it was built on — see #42.

The reason matters here rather than just the fact. Game::run_chain drives a local DiffFactor enum directly and has never gone through Schedule. So the trait was not an extension point that this issue would have extended; it was one nothing had ever plugged into, including the engine itself. Implementing this as written would have meant first wiring run_chain through an abstraction it does not use, in the hot loop, with bit-exactness obligations from tests/determinism.rs and the numerical goldens.

Two measurements from #42 that bear on whether this is still worth doing at all:

The within-game EP loop converges in 1 to 8 iterations against a cap of 30, flat in team count — measured on even and deliberately hostile uneven fields up to 20 teams. Whatever a different schedule buys, it is not iterations in the inner loop.
The Damped half of the original schedule plan already shipped, as ConvergenceOptions::alpha, inside that hand-rolled loop.

None of that rules out the parallelism idea — a barrier merge is about concurrency, a different axis from convergence rate, and #5's numbers are the relevant ones there. But it does mean this issue needs redesigning against the engine as it actually is, not reopened against a trait that is gone. Worth rewriting or closing rather than leaving it pointing at removed types.

Related: #5 (rayon default-on) remains blocked on the same measurement question.

Heads-up: this issue's premise no longer holds after `633a503`. It proposes a synchronous-EP **`Schedule`** with snapshot and barrier merge. The `Schedule` trait has been removed, along with `EpsilonOrMax`, `ScheduleReport`, `Game::custom`, and the `Factor` / `BuiltinFactor` / `RankDiffFactor` / `TeamSumFactor` machinery it was built on — see #42. The reason matters here rather than just the fact. `Game::run_chain` drives a local `DiffFactor` enum directly and **has never gone through `Schedule`**. So the trait was not an extension point that this issue would have extended; it was one nothing had ever plugged into, including the engine itself. Implementing this as written would have meant first wiring `run_chain` through an abstraction it does not use, in the hot loop, with bit-exactness obligations from `tests/determinism.rs` and the numerical goldens. Two measurements from #42 that bear on whether this is still worth doing at all: - The within-game EP loop converges in **1 to 8 iterations against a cap of 30**, flat in team count — measured on even and deliberately hostile uneven fields up to 20 teams. Whatever a different schedule buys, it is not iterations in the inner loop. - The `Damped` half of the original schedule plan already shipped, as `ConvergenceOptions::alpha`, inside that hand-rolled loop. None of that rules out the parallelism idea — a barrier merge is about *concurrency*, a different axis from convergence rate, and #5's numbers are the relevant ones there. But it does mean this issue needs redesigning against the engine as it actually is, not reopened against a trait that is gone. Worth rewriting or closing rather than leaving it pointing at removed types. Related: #5 (rayon default-on) remains blocked on the same measurement question.

logaritmisk commented

2026-09-08 00:18:10 +00:00

Closing, per the reasoning above.

To leave a clean record of what was decided rather than just that it was:

The Schedule trait this issue would have implemented is gone as of 633a503 (#42), and it was never an extension point — Game::run_chain has always driven a local DiffFactor enum directly.
Doing this as written would have meant first wiring run_chain through an abstraction it does not use, in the hot loop, against the bit-exactness obligations in tests/determinism.rs and the numerical goldens.
The within-game EP loop converges in 1 to 8 iterations against a cap of 30, flat in team count, so there is no convergence headroom for an alternative schedule to reclaim.

What would justify reopening this as a new issue: a measured case where cross-slice concurrency pays off. A snapshot-and-barrier merge is about parallelism, which is a different axis from convergence rate and is not refuted by any of the above. #5 holds the relevant numbers — T3 measured 1.0x on realistic workloads against a >=2x target — and any redesign should start from those rather than from this issue's shape.

Not reopening this one, because a rewrite against the current engine shares nothing with the text here except the idea.

Closing, per the reasoning above. To leave a clean record of what was decided rather than just that it was: - The `Schedule` trait this issue would have implemented is gone as of `633a503` (#42), and it was never an extension point — `Game::run_chain` has always driven a local `DiffFactor` enum directly. - Doing this as written would have meant first wiring `run_chain` through an abstraction it does not use, in the hot loop, against the bit-exactness obligations in `tests/determinism.rs` and the numerical goldens. - The within-game EP loop converges in 1 to 8 iterations against a cap of 30, flat in team count, so there is no convergence headroom for an alternative schedule to reclaim. **What would justify reopening this as a new issue**: a measured case where *cross-slice* concurrency pays off. A snapshot-and-barrier merge is about parallelism, which is a different axis from convergence rate and is not refuted by any of the above. #5 holds the relevant numbers — T3 measured 1.0x on realistic workloads against a >=2x target — and any redesign should start from those rather than from this issue's shape. Not reopening this one, because a rewrite against the current engine shares nothing with the text here except the idea.

logaritmisk closed this issue

2026-09-08 00:18:14 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: logaritmisk/trueskill-tt#6

Opt-in synchronous-EP Schedule with snapshot + barrier merge #6

Why

Current state

Scope

Acceptance

Related

Opt-in synchronous-EP `Schedule` with snapshot + barrier merge #6