Determinism hooks: per-thread MyRandom seeding + order-stable zone preparation#11285
Determinism hooks: per-thread MyRandom seeding + order-stable zone preparation#11285Tyrathalis wants to merge 1 commit into
Conversation
…teration in zone preparation Two small changes that make seeded simulation deterministic across concurrent games and across JVMs. Builds on the determinism patch carried in witchesofthehill/forge (d658cbc) by the manabrew project, with two refinements noted below. - MyRandom: the random source becomes an InheritableThreadLocal. setRandom() (already documented as the deterministic-simulation hook) now seeds the calling thread's stream, so concurrent headless games in one JVM cannot contaminate each other's sequences. Refinement over the manabrew patch: Inheritable, so threads spawned from a seeded thread (AI evaluation helpers) continue the parent's stream instead of silently falling back to an unseeded one. Single-threaded behavior unchanged. - Match.preparePlayerZone: iterate the deck section in sorted order. CardPool's backing map does not guarantee iteration order, so for a fixed seed the pre-shuffle library order, the random-foil roll sequence, and card id assignment order could vary across JVMs. Refinement over sorting the finished list: sorted iteration also fixes the RNG-consumption and id-assignment order, making zone preparation a pure function of (deck, seed). Tests: seeded-stream isolation between threads, child-thread stream inheritance, and insertion-order-invariant + cross-JVM-stable library preparation. Portions authored with an AI assistant (Claude), reviewed by a human. Co-authored-by: Emanuele <emadivizio@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
I wonder if I need to see if |
|
I don't think InheritableThreadLocal.withInitial exists. ThreadLocal.withInitial does exist, but there isn't an Inheritable version, which is why the patch uses the anonymous-subclass initialValue() form. It looks like there's currently a tradeoff here with ThreadUtil's pooled threads. Inheritance binds at thread creation, and cached/work-stealing pool threads are reused across games, so a pool thread either never sees the seed (plain) or can carry a stale game's Random (inheritable, if the thread was created mid-game). We went with making it inheritable, since that covers dedicated new Thread spawns and first-use pool growth from a seeded thread, and because our harness (and manabrew's) consumes randomness on the seeding thread anyway. But we could definitely switch to ThreadLocal.withInitial if you prefer it for maintainability reasons, just let me know. In the longer run, it probably makes sense to move this in-Game, into a game.getRandom(), along the lines of #11260's per-Game id counters. I didn't want to scope-creep this PR, but I'm happy to move in that direction if preferred. |
Draft for the determinism conversation with @Hanmac (#11260) and the manabrew project (Discord #ai-plotting) — two small, separately revertable changes that make seeded simulation deterministic across concurrent games and across JVMs. Re-authors the determinism patch carried in witchesofthehill/forge (
d658cbc757, credited as co-author) with two refinements. Intentionally excludes per-Game ID counters (#11260 is the better design and is maintainer-led) andImageKeys.clearCaches()(useful, but not determinism — can be its own PR).1.
MyRandom: the random source becomes anInheritableThreadLocal.setRandom()— already documented as the deterministic-simulation hook — now seeds the calling thread's stream, so concurrent headless games in one JVM can't contaminate each other's sequences. Refinement over the manabrew patch: inheritable, so threads spawned from a seeded thread (AI evaluation helpers) continue the parent's stream rather than silently falling back to an unseeded one. Single-threaded behavior is unchanged. Design question for review:InheritableThreadLocalvs plainThreadLocal— we chose inheritable so seeding survives helper-thread spawns; happy to hear if there's a spawn pattern where inheritance is wrong.2.
Match.preparePlayerZone: iterate the deck section in sorted order.CardPool's backing map doesn't guarantee iteration order, so for a fixed seed the pre-shuffle library order, the random-foil roll sequence, and the card-id assignment order can all vary across JVMs. Refinement over sorting the finished list: sorting the iteration also fixes the RNG-consumption and id-assignment order, making zone preparation a pure function of (deck, seed) — the id-assignment half complements #11260's same-construction-order ⇒ same-ids invariant. Note for anyone with seed-pinned game corpora: behavior-invariant but trajectory-changing for a given seed.Tests (3 new, all green; full forge-gui-desktop suite green, 289 tests): seeded-stream isolation between sibling threads; child-thread stream inheritance; library preparation invariant under CardPool insertion order and stable cross-JVM (sorted pre-shuffle order asserted directly).
Fork-fidelity verification: we ran this branch through our forkcheck harness (mid-game fork → digest-identical state → deterministic seeded completion under a model-driven bridge, the gate built for #11203): twin determinism 40/40, 0 static mismatches, 0 transport failures — no fidelity regression.
Portions authored with an AI assistant (Claude), reviewed by a human.
🤖 Generated with Claude Code