Repository navigation
Conversation
Row i of run_batch, run_replicates and parameter_scan was seeded base + i, so an ensemble from seed 1 was the one from seed 0 shifted by a row: 49 of 50 trajectories bit-identical. Each row's seed is now drawn from numpy's SeedSequence(base) (derived_seeds), distinct within a call and 31 bits wide, and stamped on Result.seed as before. Seeded ensembles change; seed=None, single runs and ODE do not. Three PSA mean tests bounded the mean by 5 binomial SEs, a fifth of PSA's real spread (sd 218 for 46.5); they now use the sample's SE.
This branch has not been deployed
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
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.
Closes #1365.
What was wrong
Row i of
run_batch,run_replicatesandparameter_scanwas seededbase_seed + i. Sorun_replicates(50, seed=1)wasrun_replicates(50, seed=0)shifted by one replicate. Each ensemble on its own was fine, but ensembles from nearby bases were nearly the same samples, with no warning. On main c0877fa, with the issue's birth-death model:seed=0/seed=1[0, 1, 2, 3, …]/[1, 2, 3, 4, …]run_batchcalls, seeds 0 and 1The fix
python/bngsim/_seed.py:derived_seeds(base_seed, n). It returns the seeds of rows 0..n−1, drawn from numpy'sSeedSequence(base_seed). That stream is a hash of the base and the position, so nearby bases give unrelated seeds. Each seed is masked to 31 bits, the width_resolve_seedalready draws for the NFsim, RuleMonkey and SSA backends.The seeds of one call are distinct. The first n distinct values of the stream are taken, in order. A plain 31-bit draw repeats inside an ensemble of 10,000 about once in 40, and a repeat is a duplicated trajectory, which
base + inever produced. The stream is prefix-stable, so row i's seed does not depend on n.Where it is used. Each call site computes its row seeds once, before any row runs, so the threaded paths draw what the sequential paths do:
run_batch:row_seeds = derived_seeds(base_seed, n_sims)._run_single_batchnow takes the row's seed (row_seed), not the base plus its index.run_replicates:rep_seeds, on the sequential path and per thread.parameter_scan:scan_seeds.Result.seedis the derived seed, sorun(seed=r.seed)still repeats that row alone. The docstrings of all three methods anddocs/quickstart.mdnow describe this schedule.Behaviour change
A seeded batch, ensemble or scan gives different trajectories than before. That is the point of the change, but numbers recorded from a seeded call under
base + iwill not be reproduced.seed=None(a fresh base from entropy), singlerun(seed=...)calls, and ODE (whose batch seed only breaks event ties) are unchanged. The changelog fragment ischanged, notfixed, for this reason.Tests
test_derived_seeds.py(8; on main the file fails at import, sincederived_seedsdoes not exist):parameter_scanused the samebase + i);Result.seed;Existing tests that encoded
base + i, updated toderived_seedswhile keeping what each checks:test_seed_semantics.py:test_per_sim_seeds_are_consecutivebecomes…_are_derived_from_the_base; and "seeds within a batch are consecutive" becomes "are distinct".test_ssa_run_replicates.py: the clone-per-replicate reference, against which the sequential and threaded paths are compared bit for bit, seeds each replicate asrun_replicatesnow does; and the exposed seeds arederived_seeds(100, N).test_ssa_reaction_stats.py::test_batch_rows_carry_the_blocks: compares row 0 with a run at its seed (rows[0].seed), not at the base.Three PSA mean tests had a standard error 4.7× too small. These are
test_sbml_psa.py::test_psa_sbml_matches_analytical,test_ssa_psa_volume.py::test_psa_v_neq_1_mean_matches_analyticaland::test_psa_v_eq_1_unchanged. They bounded the PSA ensemble mean by 5 SEs of the exact SSA's binomial spread, √(N·p·q) = 46.5. PSA's leaps spread the population far wider: I measured a standard deviation of 218 over 4 bases × 500 replicates. So "5·SE" was about one of PSA's SEs, and each test passed or failed with its seeds. Reseeded, all three missed by 17.2 against a bound of 16.4. PSA is not biased: over those 2,000 replicates its mean is 6843.3 against 6839.4, z = 0.8. The tests now take the SE from the sample (std(ddof=1)/√n), with the measurement in a comment.Mutations
derived_seedsreturnsbase + i(main's schedule)parameter_scanleft onbase + irun_batchleft onbase + iMeasured
Full suite (macOS arm64, main c0877fa + this branch): 16,420 passed and 4 failed. The 4 were the reaction-stats row test and the three PSA tests above, which now pass (those three files: 26 passed).
pre-commit run --all-files(new files staged) andsphinx -Wwithdocs/requirements-ci.txtare clean.