fix: engine correctness (seeded randomness, delay stranding, gate weights)#35
Merged
Conversation
- Seeded reproducibility: formula randomness (random/randomInt/pickRandom in math.js, plus Math.random in the legacy Function fallback) now draws from SimRandom via the evaluation scope, so seeded runs with random in any formula are bit-for-bit reproducible. Injected per evaluation, never baked into compiled code, since _mathCompileCache is shared across Monte Carlo trials. - Delay save/load: reset() rebuilds a delay's _queue from its colorMap (one batch per color, releasing after the node's delay), mirroring the QUEUE _fifo rebuild, so in-flight resources saved mid-run are released instead of stranding in the node forever. - Zero-weight gate outputs: _proportionalShares hands the integer remainder round-robin over positive-weight outputs only (all-zero even-split fallback kept), fixing deterministic gates and delay releases leaking units down weight-0 connections. - Fractional capacity: _fairAllocate floors `available` so a capacity like 5.5 can no longer overfill a pool to 6. - rollDice: dice count capped at 10000 (matching sampleDist's poisson guard) so 999999999d6 can't freeze the tick. - History decimation: filter retained snapshots by step phase instead of array index, keeping recording aligned with the doubled stride (no skipped samples right after each doubling). - Checkpoints: SimRandom exposes getState()/setState() and captureState/ restoreState carry the mulberry32 position, so restoring a seeded run replays the exact same stochastic draws. Adds 9 regression tests (194 total); each fails against the pre-fix engine and passes now, with and without mathjs installed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Batch 1 of the codebase review fixes: simulation-engine correctness. Every fix ships with a regression test that was verified to FAIL against the pre-fix engine (via stash), so each test genuinely pins its bug. Suite grows 185 to 194, green with and without mathjs installed (both evaluator paths).
random(),randomInt(),pickRandom()in formulas now draw from the engine's seeded SimRandom stream instead of math.js's Math.random-backed RNG; the legacynew Functionpath gets the same helpers plus aMathstand-in whoserandom()is seeded. Injected per evaluation, never baked into the shared compile cache, so Monte Carlo trials stay isolated. "Same seed, same results" now actually holds when formulas roll dicereset()rebuilds a delay node's in-flight queue from its color map (one batch per color, releasing after the node's delay), mirroring the queue rebuild, so diagrams saved mid-run (or delays given a starting amount) release their resources instead of holding them forever_fairAllocatefloors on entry)999999999d6can't freeze the tabgetState/setState(identical algorithm, state lifted out of the closure) and capture/restoreState include it, so restoring a seeded checkpoint replays the exact same draws; old snapshots without the field degrade gracefullyDeliberately not addressed here: the mid-tick order-dependence of activator conditions (conditions read live values while pulls/gates/converters mutate mid-fire). That is a simulation-semantics change that deserves its own discussion.