ENH: continue a Monte Carlo study from the root its rows were drawn with

[thc1006]

A Monte Carlo run cannot be seeded on develop: simulate() takes no random_seed. Seeding it per simulation index is not enough on its own, because an append then derives a fresh root and writes it into the same file, so a study resumed after a restart holds two lineages with nothing afterwards to say which simulation came from which. Both halves are here. Addresses #1053 and #1075.

This branch carries #1054's work as well, which is where the root comes from. That pull request is closed in favour of this one: its per-index seeding is here in full, with the append lineage built on top. The branch is squashed to one commit and rebased onto current develop, so #1181 has dropped out of the diff. What is left is 353 lines of monte_carlo.py, three test files of its own, a changelog line, a note in the stochastic guide, three added lines in the Monte Carlo notebook, and 71 lines of #1182's worker tests moved onto the claim this replaces.

Pull request type

• Code changes (bugfix, features)

Checklist

• Tests for the changes have been added (if needed)
• Docs have been reviewed and added / updated
• Lint (ruff check / ruff format --check, pylint) has passed locally
• All tests have passed locally, other than ten that need optional dependencies this machine does not have. They fail the same way on develop, listed below.
• CHANGELOG.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

On develop there is no seed to carry: random_seed does not appear in monte_carlo.py at all, so a run cannot be fixed and an append continues an unseeded one.

Per-simulation-index seeding, which was #1054 and is carried here, is not enough by itself. With it in place and nothing else, 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 with neither tellable from the other. That is what #1075 is filed against.

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 the first two rather than solving them: there is no ownership write to make atomic and no count that can drift, because there is no second file. The other two become questions about the rows themselves, which is where they can be answered. The root a row records has to be one a stream can be rebuilt from, and the two logs have to name the same study; both are checked before an append writes anything. The cost is one field per row, in rows that already carry a serialized rocket.

What this does not do

It reads both logs. Every output row carries a digest of the root its input row records, so an output log from another study is refused even when its indices match, and an append will not start until the two agree on the simulations they hold. What it does not do is judge a finished run: whether every simulation the run was asked for reached the logs is the completeness check that landed with #1182, which this branch now sits on top of.

It does not repair a log with a hole in it. A run that lost simulations is refused, not filled in.

Two things I have deliberately left, and would rather not decide alone on a format people will have on disk: versioning the persisted root, and representing a wide entropy losslessly for readers that parse JSON numbers as doubles.

Breaking change

• Yes

append=True onto a log written before this release is refused, because such a log carries no root and cannot be shown to be one study. Re-running it writes a log this release can continue.

Coverage

The patch showed six lines uncovered. Four belong to the append 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 came in
with the seeding work and their tests came with it: one that the serial failure
message names the same index the parallel path would, and one that drives the worker
loop in this process rather than a child. All six are on this branch now.

The documentation change is prose, inside one .. note:: and one docstring. The note
points at the custom sampler page with a :ref: that page already defines and
stochastic.rst already uses once, and build-docs is green on it.

Additional information

Verification, on a clean tree:

pytest tests/                          2580 passed, 10 failed
  the same on develop at d864d4d2      2518 passed, 10 failed
pytest rocketpy --doctest-modules        48 passed
ruff check . / ruff format --check .    clean
pylint rocketpy/ tests/ docs/           10.00/10, exit 0

The ten are the same ten on both, which is why I ran develop at the commit this branch sits on, d864d4d2. Each of them is an optional dependency this machine does not have: statsmodels and prettytable for the sensitivity tests, imageio for the ellipses one, and timezonefinder, windrose, ipywidgets and jsonpickle for the environment analysis ones.

Each mechanism is pinned by a mutation:

undone	goes red
rows stop carrying the root	33
the append stops reading it back	2
disagreeing rows pick the first instead of refusing	1
a rootless log is read as an empty one	2
a sequence seed does not reach the row	2
blank lines are counted as rows	2
a row that will not parse is let through	1
the nominal is read again on each reseed	1, and the constant-draw control survives

Counts are from this branch as it stands, over test_append_lineage.py and
test_monte_carlo_simulation_index.py, where the control is 43 passed. The last
row is measured separately, since undoing it means taking #1169's kept nominal
away rather than editing this branch. Every mutation was reverted from
git show HEAD: rather than from a copy, after one run left a file mutated and
the next took that as its baseline.

Bringing develop in was more than a conflict fix. #1169 landing made this branch's documentation note false: it said a run moves the nominal as it goes, so a flight could still differ by which worker took an index, and closing that is what #1169 did. The four cases it leaves outside the rule are decided by the index too, so the note is rewritten and moved to the end of the component seeding section that #1170 added. The existing split test could not have caught it, since every stochastic input in its fixture is given an explicit nominal and nothing reads one off the wrapped object. The new test uses a bare standard deviation, which is the form that does. It measures index 3 at an elevation of 1400.221102854623 whether it is reached through 0, 1 and 2 or taken on its own. Take the kept nominal away and the two stop agreeing, 1367.1672115143288 against 1367.3883143689518.

#1182 landing turned up a second one that neither branch could see alone. Its worker tests drive __sim_producer with a stand-in monitor, and that stand-in offers keep_simulating() and increment(), which this branch replaces with one claim; its _run also passes the per-worker seed the producer no longer takes. Nineteen of them went red on the rebase. They move to the claim here, and one of them changes meaning rather than shape: seeding runs after the claim now instead of once above the loop, so a model that will not reseed is reported against the index it was seeding for rather than against worker startup. The test that covered the old wording asserts the new one, and a separate test still covers a failure before any index is claimed.

Three things came out of reviewing this against itself, and all three are in:

• a log whose rows carry no root was read as empty, so an append onto one from an earlier release produced exactly the two lineages in one file that this is for. Measured before the fix: four rows, two distinct roots.
• the check held one record per row in memory to compare them. It reads one at a time now and compares with the first.
• it ran before the working-log refusal, so a .csv was reported as a row that could not be read rather than as a format simulate cannot use. BUG: refuse to run a Monte Carlo over a results file it cannot write #1161's message goes first now.

[thc1006]

cc @zuorenchen, since this sits next to your stochastic_flight work.

Updating this, because what it originally said has been overtaken. The series it listed has landed: #1181, #1169, #1170 and #1174 are on develop, and #1054 is closed in favour of this branch, which carries its per-simulation-index seeding in full. This one and #1182 are what is left.

What it does: an append continues the root its rows were drawn with, rather than quietly starting a second one in the same file.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.58%. Comparing base (d864d4d) to head (c448dd0).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1187      +/-   ##
===========================================
+ Coverage    91.50%   91.58%   +0.08%     
===========================================
  Files          131      131              
  Lines        17723    17821      +98     
===========================================
+ Hits         16217    16321     +104     
+ Misses        1506     1500       -6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Gui-FernandesBR]

@thc1006 conflicts must be solved before reviewing

have you tired stacked PRs before? See: https://docs.github.com/en/pull-requests/how-tos/stacked-pull-requests

[thc1006]

Conflicts are gone. The branch is squashed to one commit and rebased onto current develop, so the history here is one change rather than twenty plus two merges of develop.

On the stacking, since you asked: both of mine were stacked and neither is now. #1182 sat on #1181, which landed. This one sat on #1054, and I closed #1054 rather than keep the chain going, because this branch is a superset of it. The per-index seeding is here in full and the append lineage sits on top of it. Both target develop directly now.

The merge was not mechanical, so one thing is worth flagging. #1169 landing made a sentence in this branch's documentation false. The note said a run moves the nominal as it goes, so a flight could still differ by which worker took an index. That is exactly what #1169 closed, and the four cases it leaves outside the rule turn out to be decided by the index too: a component position is read from a component the run never writes back to, and select_ensemble_member rebuilds the wind from the member's own arrays rather than scaling what is already there. I rewrote the note and moved it to the end of the component seeding section, which #1170 closes by saying per-simulation reseeding is what this work adds.

The existing split test could not have caught that. Every stochastic input in its fixture is given an explicit nominal, so nothing reads one off the wrapped object, and taking the kept nominal away leaves all five of those tests green. I added one that uses a bare standard deviation, which is the form that does read one, with a control that survives.

The numbers are the test's own, so they are reproducible by running it: index 3 draws an elevation of 1400.221102854623 whether it is reached through 0, 1 and 2 or taken on its own, and taking the kept nominal away leaves 1367.1672115143288 against 1367.3883143689518. The commit message quotes a standalone probe with a nominal of 1000 instead, which is where 1002.211028546 and 671.672115143 come from.