ENH: continue a Monte Carlo study from the root its rows were drawn with - #1187
Open
thc1006 wants to merge 11 commits into
Open
ENH: continue a Monte Carlo study from the root its rows were drawn with#1187thc1006 wants to merge 11 commits into
thc1006 wants to merge 11 commits into
Conversation
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>
RocketPyEncoder records hash(obj) beside every serialized Function, and
that is the object's identity in the process that wrote the row. Two
runs sharing memory agree on it and two that do not, differ, so the
comparison was answering a question about the start method rather than
about the seed.
Windows uses spawn, and both its legs failed on exactly those fields.
Reproduced on Linux with set_start_method("spawn").
Every hash is dropped at any depth before comparing now. This does not
make the split-independence test pass under spawn: with the identity
gone it still differs on power_off_drag and power_on_drag, which is a
separate and so far unexplained difference.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The Windows legs of this branch hung and then reported a mismatch on power_off_drag and power_on_drag, and I read that as the guarantee failing under spawn. It was the test. Two fields a serialized Function carries belong to the writer rather than to the draw. hash is the object's identity in that process. A callable source is its pickle, and the same callable pickles to different bytes in a spawned child: measured on one drag curve, the parent and a forked child agree and a spawned child does not. Both of those drag curves are declared None on the fixture, so nothing varies them and neither field ever carried a draw. Measured with the models seeded by hand, no per-index seeding in the way, one seed: the parent, a forked child and a spawned child all build the same rocket, mass to the last digit. So the guarantee does hold on the start method Windows uses, and there is now a test that says so rather than an assumption. Removing the per-index seeding still turns three of these red, so dropping those two fields has not made the comparison vacuous. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Contributor
Author
|
cc @zuorenchen, since this sits next to your TL;DR: this is the last of six that came out of splitting #1054. It is draft and stacked on #1054, so nothing here needs your time yet. If you only look at one, make it #1181. The series, and what each is for:
All six are on current |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1187 +/- ##
===========================================
+ Coverage 89.96% 90.69% +0.72%
===========================================
Files 131 131
Lines 17527 17590 +63
===========================================
+ Hits 15769 15953 +184
+ Misses 1758 1637 -121 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The serial numbering only reaches whoever ran it through that message, and the producer loop is otherwise only ever watched from another process. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The workflow that would have written it has not run since RocketPy-Team#1112, which is what RocketPy-Team#1173 is about. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
An append started a second root and wrote it into the same file, so a study resumed after a restart held two lineages and nothing afterwards could say which simulation came from which. Addresses RocketPy-Team#1075. Every input row carries the root that drew it. An append reads it back and continues, so resuming needs no seed and a fresh object over the same files is the ordinary way to do it. A seed that disagrees with the rows is refused rather than mixed in, and a log whose rows disagree with each other is refused rather than resolved, since that is two studies and continuing either buries the other. In the rows rather than in a file beside them. A sidecar cannot be shown to belong to a log: names and counts match by coincidence, and a copy taken from another study passes every check that reads only itself. There is no ownership write to make transactional, no count that can disagree with the rows, and no schema to coerce, because there is no second file. Rows that carry no root are refused rather than read as an empty log. They are how a study written before this release looks, and taking them for an empty one starts a second lineage in the file, which is the failure this exists to prevent. Read one row at a time and compared with the first, so a long study is not held in memory to be checked, and run after the working-log refusal so a .csv is reported as a format it cannot use rather than as a row that cannot be read. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
A sequence seed reaches the row whole, a blank line is not a row, and a row that will not parse is refused rather than read as an empty log. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The seed docstring still pointed at RocketPy-Team#1075 as future work and the stochastic guide still asked for the same seed again; this branch is what changed both. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
thc1006
force-pushed
the
enh/append-continues-the-same-stream
branch
from
August 26, 2026 00:45
b238a13 to
ba3bfd3
Compare
thc1006
marked this pull request as ready for review
August 26, 2026 03:20
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.
An append starts a second root and writes it into the same file, so a study resumed after a restart holds two lineages and nothing afterwards can say which simulation came from which. Addresses #1075.
Stacked on #1054, which is where the root comes from. This branch carries its commits, so the diff shown here includes them: the work of this pull request is 104 lines of
monte_carlo.py, one test file, a changelog line and a paragraph of the stochastic guide. Draft until #1054 lands, and glad to rebase then.Pull request type
Checklist
ruff check/ruff format --check,pylint) has passed locallyCHANGELOG.md— one line, since one pull request is one line here and the workflow that would have written it has not run since CI: run the changelog job for pull requests from forks #1112 (BUG: the changelog workflow stopped running, and CHANGELOG.md is 37 merged pull requests behind #1173).Current behavior
simulate(append=True)derives a fresh root and carries on writing into the file it was given. Nothing records which root produced which row, so the file ends up describing two studies and neither can be told from the other afterwards.New behavior
Every input row carries the root that drew it. An append reads it back and continues from it, so resuming needs no seed, and a fresh object over the same files is the ordinary way to do it. A seed that disagrees with the rows is refused rather than mixed in. A log whose rows disagree with each other is refused rather than resolved, since that is two studies already and continuing either buries the other.
Rows that carry no root at all are refused as well, rather than read as an empty log. That is how a study written before this release looks, and taking it for an empty one starts the second lineage this exists to prevent. A genuinely empty log still starts normally, which has a test of its own.
Why the root is in the rows
The obvious alternative is a small file written beside the log. I built that first, in the branch this came out of, and it grew four problems with one root between them: a sidecar cannot be shown to belong to a log.
Names and counts match by coincidence. A copy taken from another study passes every check that reads only itself. The write that claims ownership has to be transactional with the log replacement. And the count it carries can drift from the rows it describes.
Putting the root in the rows removes all four rather than solving them. There is no ownership write to make atomic, no count that can drift, no schema to validate, and nothing to bind, because there is no second file. The cost is one field per row, in rows that already carry a serialized rocket.
What this does not do
It reads the input log. An output log belonging to a different study is not caught here; comparing the two index sets is #1182's completeness check, and the two together cover it.
It does not repair a log with a hole in it. A run that lost simulations is refused, not filled in.
Breaking change
append=Trueonto a log written before this release is refused instead of silently starting a second lineage. Re-running the study writes a log this release can continue.Coverage
The patch showed six lines uncovered. Four are this branch's and now have tests: a
sequence seed reaching the row whole, a blank line not being a row, and a row that
will not parse being refused rather than read as an empty log. The other two belong
to #1054 underneath and are covered there, by a test that the serial failure message
names the same index the parallel path would, and one that drives the worker loop in
this process instead of a child.
The only documentation change is prose, inside a
.. note::that already existed andinside a docstring. There are no new directives or references, so I checked the
indentation by hand rather than building the docs locally.
Additional information
Verification, on a clean tree:
Each mechanism is pinned by a mutation:
Three things came out of reviewing this against itself, and all three are in:
.csvwas reported as a row that could not be read rather than as a formatsimulatecannot use. BUG: refuse to run a Monte Carlo over a results file it cannot write #1161's message goes first now.