ENH: reproducible Monte Carlo via per-simulation-index seeding - #1054
ENH: reproducible Monte Carlo via per-simulation-index seeding#1054thc1006 wants to merge 4 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1054 +/- ##
===========================================
+ Coverage 84.57% 90.55% +5.98%
===========================================
Files 131 131
Lines 17527 17551 +24
===========================================
+ Hits 14824 15894 +1070
+ Misses 2703 1657 -1046 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
The seeding logic is unit-tested in The lines codecov still shows uncovered are all in the parallel path: |
0d37ed6 to
761c092
Compare
phmbressan
left a comment
phmbressan
left a comment
There was a problem hiding this comment.
The implementation is very clear and throughout, nice work.
The explanation on the concepts behind per index seeding (both in the issue and PR description) were rather helpful. I agree having reproducible results was an issue with the parallel per worker seeding.
Regarding the decisions on parameter naming, I agree with most of the decisions taken here. Moreover, the rng attribute is well docstringed, so it shouldn't be a matter of confusion to the user.
@MateusStano could you give your two cents on the changes here before we proceed with a merge?
Addresses review feedback on RocketPy-Team#1054. Parallel workers claimed the next index with an unlocked keep_simulating() + increment(), so near the end of a run two workers could both pass the count < n check and then claim sim_idx == n; the per-index child_seeds lookup turned that into an IndexError (before, it only wrote one extra record). Move the claim into a _claim_next_index helper that holds the shared mutex across the check and the increment, so each index is handed out once and the counter never overshoots. A deterministic unit test (a barrier plus a widened check-to-increment window) over-claims and fails if the lock is dropped. __root_seed_sequence returned the caller's SeedSequence, and spawn() advances its child counter, so passing the same object to simulate() twice produced different children. Copy it from its full state instead, which leaves the caller untouched and keeps repeated calls reproducible. Also drop Generator/BitGenerator from the accepted types: a stateful generator is not a seed, and reducing it to its underlying SeedSequence ignores how far it has been consumed. random_seed now takes an int, a sequence of ints, or a SeedSequence. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Addresses review feedback on RocketPy-Team#1054. Parallel workers claimed the next index with an unlocked keep_simulating() + increment(), so near the end of a run two workers could both pass the count < n check and then claim sim_idx == n; the per-index child_seeds lookup turned that into an IndexError (before, it only wrote one extra record). Move the claim into a _claim_next_index helper that holds the shared mutex across the check and the increment, so each index is handed out once and the counter never overshoots. A deterministic unit test (a barrier plus a widened check-to-increment window) over-claims and fails if the lock is dropped. __root_seed_sequence returned the caller's SeedSequence, and spawn() advances its child counter, so passing the same object to simulate() twice produced different children. Copy it from its full state instead, which leaves the caller untouched and keeps repeated calls reproducible. Also drop Generator/BitGenerator from the accepted types: a stateful generator is not a seed, and reducing it to its underlying SeedSequence ignores how far it has been consumed. random_seed now takes an int, a sequence of ints, or a SeedSequence. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
9c020b6 to
3e22729
Compare
Addresses review feedback on RocketPy-Team#1054. Parallel workers claimed the next index with an unlocked keep_simulating() + increment(), so near the end of a run two workers could both pass the count < n check and then claim sim_idx == n; the per-index child_seeds lookup turned that into an IndexError (before, it only wrote one extra record). Move the claim into a _claim_next_index helper that holds the shared mutex across the check and the increment, so each index is handed out once and the counter never overshoots. A deterministic unit test (a barrier plus a widened check-to-increment window) over-claims and fails if the lock is dropped. __root_seed_sequence returned the caller's SeedSequence, and spawn() advances its child counter, so passing the same object to simulate() twice produced different children. Copy it from its full state instead, which leaves the caller untouched and keeps repeated calls reproducible. Also drop Generator/BitGenerator from the accepted types: a stateful generator is not a seed, and reducing it to its underlying SeedSequence ignores how far it has been consumed. random_seed now takes an int, a sequence of ints, or a SeedSequence. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
3e22729 to
6bf8bb6
Compare
|
@MateusStano friendly ping when you have a moment. Both points from your last pass are addressed: the parallel index claim now holds the shared mutex across the check-and-increment (with a deterministic test that goes red if the lock is removed), and a supplied |
6bf8bb6 to
c529d0a
Compare
Addresses review feedback on RocketPy-Team#1054. Parallel workers claimed the next index with an unlocked keep_simulating() + increment(), so near the end of a run two workers could both pass the count < n check and then claim sim_idx == n; the per-index child_seeds lookup turned that into an IndexError (before, it only wrote one extra record). Move the claim into a _claim_next_index helper that holds the shared mutex across the check and the increment, so each index is handed out once and the counter never overshoots. A deterministic unit test (a barrier plus a widened check-to-increment window) over-claims and fails if the lock is dropped. __root_seed_sequence returned the caller's SeedSequence, and spawn() advances its child counter, so passing the same object to simulate() twice produced different children. Copy it from its full state instead, which leaves the caller untouched and keeps repeated calls reproducible. Also drop Generator/BitGenerator from the accepted types: a stateful generator is not a seed, and reducing it to its underlying SeedSequence ignores how far it has been consumed. random_seed now takes an int, a sequence of ints, or a SeedSequence. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
|
Heads up that this changed enough since the last look to be worth a fresh pass rather than merging on the earlier approval. @MateusStano @phmbressan when you have a moment. What is new since the review:
Both earlier concerns are still handled: the parallel claim holds the mutex across the check and the increment, and a supplied |
c529d0a to
e5ba940
Compare
|
@MateusStano @phmbressan a follow-up pass turned up a few more things worth fixing, so I have pushed them and would appreciate another look when you have time. Since your reviews:
I also marked the earlier threads resolved. The race and the SeedSequence copy are both fixed in the current code, and the dangling-files question checked out: the run writes only under A few larger items from the same review are better as their own issues, so I opened #1075 (append continuation), #1076 (a full parallel test under spawn and forkserver) and #1077 (a seed for |
5a9c119 to
da2ba5c
Compare
|
A quick note on the red CI here, so it isn't mistaken for a regression from this change: the failing jobs crash in Re-running usually clears it. Happy to help look at the flaky animation tests on their own if that would be useful. Filed #1078 to track the flaky animation tests. |
Addresses review feedback on RocketPy-Team#1054. Parallel workers claimed the next index with an unlocked keep_simulating() + increment(), so near the end of a run two workers could both pass the count < n check and then claim sim_idx == n; the per-index child_seeds lookup turned that into an IndexError (before, it only wrote one extra record). Move the claim into a _claim_next_index helper that holds the shared mutex across the check and the increment, so each index is handed out once and the counter never overshoots. A deterministic unit test (a barrier plus a widened check-to-increment window) over-claims and fails if the lock is dropped. __root_seed_sequence returned the caller's SeedSequence, and spawn() advances its child counter, so passing the same object to simulate() twice produced different children. Copy it from its full state instead, which leaves the caller untouched and keeps repeated calls reproducible. Also drop Generator/BitGenerator from the accepted types: a stateful generator is not a seed, and reducing it to its underlying SeedSequence ignores how far it has been consumed. random_seed now takes an int, a sequence of ints, or a SeedSequence. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
9ed65ae to
7053db2
Compare
|
I checked the current head,
The remaining blocker appears to be repository state rather than an observed test failure. @thc1006, could you rebase or resolve the conflict against the current Local command: Environment: RocketPy source at the head above; Python 3.12.6; NumPy 2.5.2; pytest 9.1.1; macOS 26.5.2 arm64. |
| Default is None. | ||
| random_seed : int, numpy integer, sequence of ints, or SeedSequence, optional | ||
| Root seed for the run. When provided, the mapping from simulation | ||
| index to sampled inputs is reproducible and identical across serial |
There was a problem hiding this comment.
This looks good to me overall. One small wording point: the seed tree guarantees reproducibility of the sampled inputs, but not necessarily the complete flight trajectory because some process-global randomness remains outside it. Otherwise, the architecture and test coverage look solid.
Addresses review feedback on RocketPy-Team#1054. Parallel workers claimed the next index with an unlocked keep_simulating() + increment(), so near the end of a run two workers could both pass the count < n check and then claim sim_idx == n; the per-index child_seeds lookup turned that into an IndexError (before, it only wrote one extra record). Move the claim into a _claim_next_index helper that holds the shared mutex across the check and the increment, so each index is handed out once and the counter never overshoots. A deterministic unit test (a barrier plus a widened check-to-increment window) over-claims and fails if the lock is dropped. __root_seed_sequence returned the caller's SeedSequence, and spawn() advances its child counter, so passing the same object to simulate() twice produced different children. Copy it from its full state instead, which leaves the caller untouched and keeps repeated calls reproducible. Also drop Generator/BitGenerator from the accepted types: a stateful generator is not a seed, and reducing it to its underlying SeedSequence ignores how far it has been consumed. random_seed now takes an int, a sequence of ints, or a SeedSequence. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
7053db2 to
3b4eaa8
Compare
Addresses review feedback on RocketPy-Team#1054. Parallel workers claimed the next index with an unlocked keep_simulating() + increment(), so near the end of a run two workers could both pass the count < n check and then claim sim_idx == n; the per-index child_seeds lookup turned that into an IndexError (before, it only wrote one extra record). Move the claim into a _claim_next_index helper that holds the shared mutex across the check and the increment, so each index is handed out once and the counter never overshoots. A deterministic unit test (a barrier plus a widened check-to-increment window) over-claims and fails if the lock is dropped. __root_seed_sequence returned the caller's SeedSequence, and spawn() advances its child counter, so passing the same object to simulate() twice produced different children. Copy it from its full state instead, which leaves the caller untouched and keeps repeated calls reproducible. Also drop Generator/BitGenerator from the accepted types: a stateful generator is not a seed, and reducing it to its underlying SeedSequence ignores how far it has been consumed. random_seed now takes an int, a sequence of ints, or a SeedSequence. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
3b4eaa8 to
f1e0a64
Compare
Addresses review feedback on RocketPy-Team#1054. Parallel workers claimed the next index with an unlocked keep_simulating() + increment(), so near the end of a run two workers could both pass the count < n check and then claim sim_idx == n; the per-index child_seeds lookup turned that into an IndexError (before, it only wrote one extra record). Move the claim into a _claim_next_index helper that holds the shared mutex across the check and the increment, so each index is handed out once and the counter never overshoots. A deterministic unit test (a barrier plus a widened check-to-increment window) over-claims and fails if the lock is dropped. __root_seed_sequence returned the caller's SeedSequence, and spawn() advances its child counter, so passing the same object to simulate() twice produced different children. Copy it from its full state instead, which leaves the caller untouched and keeps repeated calls reproducible. Also drop Generator/BitGenerator from the accepted types: a stateful generator is not a seed, and reducing it to its underlying SeedSequence ignores how far it has been consumed. random_seed now takes an int, a sequence of ints, or a SeedSequence. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
0f48613 to
81d3e58
Compare
|
This is a quite large PR... I wonder if it would be easy and possible to split it into smaller PRs... If if becomes a too hard task, no problem, just give us more time so we can properly review it |
|
TL;DR: splitting it. I am putting this back to draft now so nobody spends review time on the current head. Nothing gets thrown away, and I will link the series here before I rewrite the branch. Thanks for saying it, and thanks for offering the alternative of just taking more time. Splitting is the better answer. You were right to ask, and I should have stopped adding to this head sooner. One piece of good news about the size: a fair amount of what accumulated here has landed separately while this sat. The seeded list choice, the per-sampler What I plan to lift out, each based on current
#1054 then keeps only the @ting-hong-shieh thanks for checking the head and reporting the conflict against I will ask for review on one at a time rather than sending five at once. No action needed from either of you until the first one is up. |
|
Two of the split are up and green: #1169, a stable nominal across reseeding, and #1170, one random stream per rocket component. The series, which I will fill in as each goes up:
Neither of the two depends on the other, and they have been merged into a throwaway tree on The head being split from is kept at |
_set_stochastic re-validates every declared input, and validation reads the nominal off the wrapped object. create_object writes the sampled value back onto that same object on purpose, so re-reading it on a reseed took one simulation's output as the next one's nominal: a wind factor compounded 10 -> 8.576 -> 7.355 -> 6.308 under a single fixed seed, and a plain scalar spec drifted the same way. Read the nominal once and keep it. Containers are copied on the way in, so writing through the wrapped object cannot reach it either. A component position arrives through an injected getter, reads an attribute nothing writes back to, and shares one name across every component, so those are read live rather than cached. Extracted from RocketPy-Team#1054. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
_set_stochastic handed the same seed to the rocket body and to every surface, motor, rail button and parachute, so two components built from one spec drew identical values: a main and a drogue with the same cd_s and lag spec drew the same cd_s and the same lag, every time, and a study of both was a study of one counted twice. Air brakes were worse. They are built and sampled in create_object and were not in the reseed at all, so their values came from wherever the generator had been left rather than from the seed: 0.683, then 0.586, then 0.488 for one seed asked three times. Each component now takes its own child of a SeedSequence root, spawned in a fixed order so one seed still reproduces the whole rocket. The collections are named in one place and checked against create_object's own source, since the collection no fixture populates is the one that gets missed. Extracted from RocketPy-Team#1054. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
simulate() returned normally after Ctrl-C. Both execution modes caught the interrupt and neither re-raised it, so simulate() went on to __terminate_simulation() and returned exactly as it does after a complete study. A caller could not tell a partial run from a finished one without opening the output file and counting rows. Three more holes sat next to that one, found in review: - __run_in_serial bound inputs_json inside the loop body, so Ctrl-C during the first keep_simulating() call reached the handler with nothing bound and the run died with UnboundLocalError from inside the cleanup. - _append_simulation_record rolled its inputs row back on Exception, and KeyboardInterrupt does not derive from Exception, so Ctrl-C between the two appends left a one-sided record for the reload to disagree with. - The parallel cleanup began only after every worker had started, so Ctrl-C during the startup loop left the already-started workers running with nobody signalling or joining them. Bind inputs_json before the loop, roll back on BaseException, cover the startup loop with the same cleanup path over a started-workers list, and re-raise in both handlers. Catch the interrupt in simulate() so __terminate_simulation() still runs before it leaves: it reloads the logs through the file setters, and set_num_of_loaded_sims is what the documented append=True continuation reads. The shutdown join stays unbounded, and the docstring now says so: the interrupt propagates once every worker finishes the simulation it is in and exits on its own. A worker stuck inside one simulation blocks the interrupt as it already blocked the run; killing it here could tear a half-written row into logs it holds the mutex for, and the bounded fleet shutdown belongs to RocketPy-Team#1054. The ordinary exception path is unchanged. Add eight regression tests covering both modes, the early interrupt, the preserved rows, the reload, an interrupted run continued with append=True, the between-appends rollback, and the startup-loop cleanup. They run the real __terminate_simulation and assert the state it produces; all eight fail on develop and each one fails again when only its own fix is reverted.
simulate() returned normally after Ctrl-C. Both execution modes caught the interrupt and neither re-raised it, so simulate() went on to __terminate_simulation() and returned exactly as it does after a complete study. A caller could not tell a partial run from a finished one without opening the output file and counting rows. Five more holes sat next to that one, found in review and in the interrupt-point audit it prompted: - __run_in_serial bound inputs_json inside the loop body, so Ctrl-C during the first keep_simulating() call reached the handler with nothing bound and the run died with UnboundLocalError from inside the cleanup. - It also never cleared inputs_json after a successful append, so Ctrl-C landing in the progress print — or in the next keep_simulating() call — wrote the row that had just committed into the error file as though it never finished. - _append_simulation_record rolled back on Exception, and KeyboardInterrupt does not derive from Exception, so Ctrl-C between the two appends left a one-sided record. - Its rollback also only truncated the inputs file, so an interrupt inside either write left a torn partial row for the reload to die on with JSONDecodeError instead of the interrupt. - The parallel cleanup began only after every worker had started, so Ctrl-C during the startup loop left the already-started workers running with nobody signalling or joining them. Bind inputs_json before the loop and clear it after each committed append, roll both files back on BaseException best-effort, cover the startup loop with the same cleanup path over a started-workers list, and re-raise in both handlers. Catch the interrupt in simulate() so __terminate_simulation() still runs before it leaves: it reloads the logs through the file setters, and set_num_of_loaded_sims is what the documented append=True continuation reads. The shutdown join stays unbounded, and the docstring now says so: the interrupt propagates once every worker finishes the simulation it is in and exits on its own. A worker stuck inside one simulation blocks the interrupt as it already blocked the run; killing it here could tear a half-written row into logs it holds the mutex for, and the bounded fleet shutdown belongs to RocketPy-Team#1054. The ordinary exception path is unchanged. Add ten regression tests covering both modes, the early interrupt, the preserved rows, the reload, an interrupted run continued with append=True, the between-appends rollback, the torn-write rollback, the committed row staying out of the error file, and the startup-loop cleanup. They run the real __terminate_simulation and assert the state it produces; all fail on develop, and each fix was also reverted individually with only its own test failing.
simulate() returned normally after Ctrl-C. Both execution modes caught the interrupt and neither re-raised it, so simulate() went on to __terminate_simulation() and returned exactly as it does after a complete study. A caller could not tell a partial run from a finished one without opening the output file and counting rows. Five more holes sat next to that one, found in review and in the interrupt-point audit it prompted: - __run_in_serial bound inputs_json inside the loop body, so Ctrl-C during the first keep_simulating() call reached the handler with nothing bound and the run died with UnboundLocalError from inside the cleanup. - It also never cleared inputs_json after a successful append, so Ctrl-C landing in the progress print — or in the next keep_simulating() call — wrote the row that had just committed into the error file as though it never finished. - _append_simulation_record rolled back on Exception, and KeyboardInterrupt does not derive from Exception, so Ctrl-C between the two appends left a one-sided record. - Its rollback also only truncated the inputs file, so an interrupt inside either write left a torn partial row for the reload to die on with JSONDecodeError instead of the interrupt. - The parallel cleanup began only after every worker had started, so Ctrl-C during the startup loop left the already-started workers running with nobody signalling or joining them. Bind inputs_json before the loop and clear it after each committed append, roll both files back on BaseException best-effort, cover the startup loop with the same cleanup path over a started-workers list, and re-raise in both handlers. Catch the interrupt in simulate() so __terminate_simulation() still runs before it leaves: it reloads the logs through the file setters, and set_num_of_loaded_sims is what the documented append=True continuation reads. The shutdown join stays unbounded, and the docstring now says so: the interrupt propagates once every worker finishes the simulation it is in and exits on its own. A worker stuck inside one simulation blocks the interrupt as it already blocked the run; killing it here could tear a half-written row into logs it holds the mutex for, and the bounded fleet shutdown belongs to RocketPy-Team#1054. The ordinary exception path is unchanged. Add ten regression tests covering both modes, the early interrupt, the preserved rows, the reload, an interrupted run continued with append=True, the between-appends rollback, the torn-write rollback, the committed row staying out of the error file, and the startup-loop cleanup. They run the real __terminate_simulation and assert the state it produces; all fail on develop, and each fix was also reverted individually with only its own test failing.
A parallel run spawns a SeedSequence per worker and passes it to environment, rocket and flight. _sampler_seed then fed it to SeedSequence(entropy=...), which takes an int or a sequence of ints, so the first worker raised TypeError before drawing anything. The call was reached only from the custom sampler reset until RocketPy-Team#1117 added the list-choice generator, which every model goes through. A real two-worker run passes at d21abde^ in 2.32s and does not finish on develop: the worker's own error path raises UnboundLocalError on inputs_json, so the parent never learns it died and the run hangs. The children of one root share their entropy and differ by spawn_key, so the value is folded through generate_state rather than read off entropy, which would put every worker on one sampler stream. Nothing is consumed, and an int or None seed keeps the stream it had. The fold lives in rocketpy.tools, since the component streams and the per-index seeding both need the same one and three copies would drift on width and word order. _sampler_seed does its own final fold through it as well rather than repeating the four lines. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The two run paths named the same simulation differently. Three of them wrote 1, 2, 3 through the serial path and 0, 1, 2 through the parallel one, so a row could not be compared with its counterpart and an index meant nothing on its own. Serial counts from zero now, which is what the parallel path already did and what append already assumed: num_of_loaded_sims counts rows, so a two-row checkpoint resumes at 2, an index the serial path never used. Existing serial results are numbered one higher than the same run would be numbered now. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
A run seeded its workers, one seed each, from fresh entropy every time, and the serial path never reseeded at all. So the same study gave different results run to run, different results in the two modes, and different results again when the worker count changed. Addresses RocketPy-Team#1053. simulate() takes random_seed now, keyword-only, and every simulation takes the child of that root belonging to its index. The child is derived directly rather than by spawning the ones before it: spawn appends n_children_spawned + i to the parent key, so rebuilding that one child reproduces it bit for bit, and a worker reaches any index from four picklable values instead of a list a million long. There is a test comparing it with spawn(n)[i] for every seed type. Measured over real flights, four simulations: serial(42) == serial(42) True serial(42) == parallel(2 workers, 42) True serial(42) == parallel(4 workers, 42) True serial(42) == serial(7) False The per-worker seed is gone rather than kept alongside, since a worker now decides nothing about sampling and how many there are cannot reach it. Appending continues the same stream when the same seed is given, since an index maps to a seed and nothing else. Nothing here checks that the caller did give the same one; persisting the root so it can be checked is RocketPy-Team#1075. Fixed-seed results change: every study is sampled from a different place. Nothing that was reproducible before stops being so, because nothing was. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
simulate() gained random_seed with no entry in its own Parameters block, and the stochastic page hands users to the MonteCarlo class without saying that a run is fixed there rather than on the models. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
696c516 to
8366640
Compare
Rewritten from current
develop. The old head is not replayed: it is kept atthc1006/RocketPy@backup/pr-1054-cf99e872, and what came out of it went up separately as #1169, #1170, #1181 and #1182.What is left here is the thing #1053 asked for and nothing else.
The other three files are the two test files and a paragraph in
docs/user/stochastic.rst.Stacked on #1181. This branch carries that commit, so the diff shown here includes it. Happy to rebase once it lands.
Pull request type
Checklist
ruff check/ruff format --check,pylint) has passed locallyCHANGELOG.md— no action needed; an LLM workflow auto-updates it after merge, though see BUG: the changelog workflow stopped running, and CHANGELOG.md is 37 merged pull requests behind #1173 for why it currently does not.Current behavior
simulate()has no seed at all. The two run paths seed differently, and one of them does not seed:So a study gives different results run to run, different results in the two modes, and different results again when the worker count changes. That is #1053.
Underneath it, the two paths do not even agree on what a simulation is called. Three simulations, measured:
An index therefore meant nothing on its own, and no index-keyed guarantee was possible before that was settled. It is the first commit here, separately revertible.
New behavior
simulate(..., random_seed=None), keyword-only. Every simulation takes the child of that root belonging to its index.The child is derived directly rather than by spawning the ones before it.
spawnappendsn_children_spawned + ito the parent key, so rebuilding that one child reproduces it bit for bit and a worker reaches any index from four picklable values instead of a list a million long. There is a test comparing it againstspawn(n)[i], for every index, for each ofint,None, a sequence, and aSeedSequence.Measured over real flights, four simulations:
The last line is the control. Without it the first three pass on a run that ignores the seed.
The per-worker seed is gone rather than kept alongside. A worker decides nothing about sampling now, so
__sim_producerno longer takes a seed and__run_in_parallelno longer spawns one per worker.pylintfinding the argument unused is what made that obvious.Addresses #1053. Not
Closes, because the default branch ismasterand a closing keyword only fires into it.Breaking change
Two, both worth stating plainly.
Fixed-seed results change, because there were none: nothing in
simulate()was reproducible before, so nothing that was reproducible stops being so. Every study is now sampled from a different place than it would have been.Serial results are numbered one lower. A three-simulation serial run wrote indices 1, 2, 3 and now writes 0, 1, 2. Anything reading those indices back sees the shift. Append already assumed zero-based, since
num_of_loaded_simscounts rows and a two-row checkpoint resumes at index 2, which the serial path never used.What this does not do
Appending continues the same stream when the same seed is given again, since an index maps to a seed and to nothing else. Nothing here records the seed, so nothing here can tell you whether a later append was given the same one. Persisting the root so that can be checked is #1075, and the docstring says so rather than implying more.
Not in here, and each already has its own place: the nominal lifetime (#1169), component streams (#1170), worker failure reporting (#1182), checkpoint and manifest work (#1075). The old branch had all of them at once, which is what made it unreviewable.
Additional information
Verification, on a clean tree:
The four
tests/unit/test_sensitivity.pyfailures on my machine are a missingstatsmodelsand fail the same way on an untoucheddevelop.Each mechanism is pinned by a mutation, and each leaves a control standing:
SeedSequenceused as givenOne mutation survived, and finding that was the point. I had written the entropy copy twice, once in
_root_seed_sequenceand once in_root_state_of. Removing either alone changed nothing, because the other still did the job; only removing both turned a test red. So one of them could not be distinguished from its own absence. Kept the one at the boundary where the value stops being the caller's, deleted the other.The copy is not decoration.
SeedSequencekeeps a sequence entropy by reference:Merged with #1169, #1170, #1182 and #1171's follow-up into a throwaway tree on
developand run there. That found two things no branch shows on its own:pylintR0915 on__sim_producer, which is under the limit on each branch and over it once both grew, and ten of #1182's tests calling the producer with the seed argument this branch removes, which git merged without a conflict marker. Both are noted for whichever lands second.