From 5a572d3ce39c91f677d0a391be371257ee78e9ae Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Tue, 18 Aug 2026 02:07:55 +0800 Subject: [PATCH 01/15] BUG: accept the seed type a Monte Carlo worker is handed 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 #1117 added the list-choice generator, which every model goes through. A real two-worker run passes at d21abde6^ 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> --- rocketpy/stochastic/stochastic_model.py | 20 ++++- rocketpy/tools.py | 11 +++ .../test_monte_carlo_parallel_runs.py | 32 ++++++++ tests/unit/stochastic/test_seed_types.py | 81 +++++++++++++++++++ 4 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 tests/unit/simulation/test_monte_carlo_parallel_runs.py create mode 100644 tests/unit/stochastic/test_seed_types.py diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index d42fb76c5..1dadb2f01 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -8,7 +8,7 @@ from rocketpy.mathutils.function import Function from rocketpy.stochastic.custom_sampler import CustomSampler -from ..tools import get_distribution +from ..tools import _seed_sequence_to_int, get_distribution def _names_as_spawn_key(input_names): @@ -41,6 +41,18 @@ def _format_number(value): return f"array of shape {np.shape(value)}" +def _seed_as_entropy(seed): + """A seed as something ``SeedSequence`` will take as entropy. + + A parallel run is handed a ``SeedSequence``, which it will not take. Any + other seed goes through untouched, so the stream an int reaches stays where + it was. + """ + if not isinstance(seed, np.random.SeedSequence): + return seed + return _seed_sequence_to_int(seed) + + def _sampler_seed(seed, input_names): """Derive a seed for one sampler, or for one group that shares a generator. @@ -54,10 +66,10 @@ def _sampler_seed(seed, input_names): # Sorted here rather than trusting the caller, so a future call site cannot # give one group two different seeds by listing its members another way. root = np.random.SeedSequence( - entropy=seed, spawn_key=_names_as_spawn_key(tuple(sorted(input_names))) + entropy=_seed_as_entropy(seed), + spawn_key=_names_as_spawn_key(tuple(sorted(input_names))), ) - words = root.generate_state(4, dtype=np.uint32) - return sum(int(word) << (32 * position) for position, word in enumerate(words)) + return _seed_sequence_to_int(root) # TODO: Stop using assert in production code. Use exceptions instead. diff --git a/rocketpy/tools.py b/rocketpy/tools.py index 0d7f1a74e..7f31f3e19 100644 --- a/rocketpy/tools.py +++ b/rocketpy/tools.py @@ -1377,6 +1377,17 @@ def euler313_to_quaternions(phi, theta, psi): return e0, e1, e2, e3 +def _seed_sequence_to_int(seed_sequence): + """Returns a ``SeedSequence`` as the 128-bit ``int`` it can be rebuilt from. + + Folded through ``generate_state`` rather than read off ``entropy``, since + the children of one root differ only by ``spawn_key``, and combined by + value so it does not depend on byte order. + """ + words = seed_sequence.generate_state(4, dtype=np.uint32) + return sum(int(word) << (32 * position) for position, word in enumerate(words)) + + def get_matplotlib_supported_file_endings(): """Gets the file endings supported by matplotlib. diff --git a/tests/unit/simulation/test_monte_carlo_parallel_runs.py b/tests/unit/simulation/test_monte_carlo_parallel_runs.py new file mode 100644 index 000000000..4ab0be440 --- /dev/null +++ b/tests/unit/simulation/test_monte_carlo_parallel_runs.py @@ -0,0 +1,32 @@ +import pytest + +from rocketpy.simulation.monte_carlo import MonteCarlo + + +@pytest.mark.parametrize("parallel", [False, True]) +def test_a_monte_carlo_run_finishes( + stochastic_environment, stochastic_calisto, stochastic_flight, tmp_path, parallel +): + # The parallel path hands each worker a SeedSequence rather than an int, and + # nothing else in the suite exercises that. A worker that dies on it is not + # reported, so this reads as a hang rather than as a failure. + # + # Built here rather than taken from the monte_carlo_calisto fixture, whose + # own filename is fixed, since `filename` is a plain attribute and the three + # working paths are settled when the object is constructed. + analysis = MonteCarlo( + filename=str(tmp_path / "study"), + environment=stochastic_environment, + rocket=stochastic_calisto, + flight=stochastic_flight, + ) + + analysis.simulate( + number_of_simulations=2, + append=False, + parallel=parallel, + n_workers=2 if parallel else None, + ) + + assert analysis.num_of_loaded_sims == 2 + assert str(tmp_path) in str(analysis.output_file) diff --git a/tests/unit/stochastic/test_seed_types.py b/tests/unit/stochastic/test_seed_types.py new file mode 100644 index 000000000..ba3d42583 --- /dev/null +++ b/tests/unit/stochastic/test_seed_types.py @@ -0,0 +1,81 @@ +import numpy as np +import pytest + +from rocketpy.stochastic.stochastic_model import ( + _names_as_spawn_key, + _sampler_seed, +) +from rocketpy.tools import _seed_sequence_to_int + + +def _a_worker_seed(index=0, workers=2): + # What MonteCarlo.__run_in_parallel spawns and hands to each worker, which + # passes it straight to environment/rocket/flight._set_stochastic. + return np.random.SeedSequence().spawn(workers)[index] + + +def test_the_seed_type_a_worker_is_handed_is_accepted(stochastic_calisto): + stochastic_calisto._set_stochastic(_a_worker_seed()) + + stochastic_calisto.create_object() + + +def test_a_parachute_derives_its_noise_seed_from_a_worker_seed( + stochastic_main_parachute, +): + stochastic_main_parachute._set_stochastic(_a_worker_seed()) + + assert stochastic_main_parachute.create_object().noise[2] is not None + + +def test_two_workers_do_not_share_a_sampler_stream(): + first, second = np.random.SeedSequence(7).spawn(2) + # They come off one root, so they carry the same entropy and differ only in + # spawn_key. Reading the entropy alone would put both on one stream. + assert first.entropy == second.entropy + + assert _sampler_seed(first, ("__list_choice__",)) != _sampler_seed( + second, ("__list_choice__",) + ) + + +def test_a_caller_seed_sequence_is_not_consumed(): + root = np.random.SeedSequence(42) + + _sampler_seed(root, ("__list_choice__",)) + + assert root.n_children_spawned == 0 + assert root.spawn(1)[0].spawn_key == (0,) + + +def test_the_same_seed_sequence_twice_gives_the_same_sampler_seed(): + root = np.random.SeedSequence(42) + + first = _sampler_seed(root, ("pressure_noise", "main")) + second = _sampler_seed(root, ("pressure_noise", "main")) + + assert first == second + + +@pytest.mark.parametrize("seed", [42, 7, [1, 2, 3]]) +@pytest.mark.parametrize("names", [("__list_choice__",), ("pressure_noise", "main")]) +def test_a_seed_that_is_not_a_sequence_reaches_numpy_untouched(seed, names): + # The control. Every fixed-seed baseline in the suite was recorded through + # this path, so anything but a SeedSequence has to arrive as it always did. + # Compared with the expression rather than with a recorded number, which + # would go red on a NumPy release instead of on a change of ours. + unchanged = np.random.SeedSequence( + entropy=seed, spawn_key=_names_as_spawn_key(tuple(sorted(names))) + ) + + assert _sampler_seed(seed, names) == _seed_sequence_to_int(unchanged) + + +def test_no_seed_still_means_no_seed(): + # None is left out above on purpose: it asks NumPy for fresh entropy, so + # two calls must not agree, and comparing one against another would be + # asserting the opposite of what an unseeded run promises. + first = _sampler_seed(None, ("__list_choice__",)) + second = _sampler_seed(None, ("__list_choice__",)) + + assert first != second From 73773f22c819edfac7c8d7f76f80d5ba9d56c215 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Tue, 18 Aug 2026 05:16:07 +0800 Subject: [PATCH 02/15] BUG: number serial simulations the way parallel numbers them 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> --- rocketpy/simulation/monte_carlo.py | 12 ++- .../test_monte_carlo_simulation_index.py | 76 +++++++++++++++++++ 2 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 tests/unit/simulation/test_monte_carlo_simulation_index.py diff --git a/rocketpy/simulation/monte_carlo.py b/rocketpy/simulation/monte_carlo.py index c2dcd4030..cec94fbc3 100644 --- a/rocketpy/simulation/monte_carlo.py +++ b/rocketpy/simulation/monte_carlo.py @@ -413,14 +413,18 @@ def __run_in_serial(self): n_simulations=self.number_of_simulations, start_time=time(), ) + sim_idx = sim_monitor.count try: while sim_monitor.keep_simulating(): - sim_monitor.increment() + # Counted from zero, as the parallel path already does. The two + # named the same simulation differently: three of them wrote + # 1, 2, 3 here and 0, 1, 2 there. + sim_idx = sim_monitor.increment() - 1 inputs_json, outputs_json = "", "" flight = self.__run_single_simulation() - inputs_json = self.__evaluate_flight_inputs(sim_monitor.count) - outputs_json = self.__evaluate_flight_outputs(flight, sim_monitor.count) + inputs_json = self.__evaluate_flight_inputs(sim_idx) + outputs_json = self.__evaluate_flight_outputs(flight, sim_idx) self._append_simulation_record(inputs_json, outputs_json) @@ -434,7 +438,7 @@ def __run_in_serial(self): f.write(inputs_json) except Exception as error: - print(f"Error on iteration {sim_monitor.count}: {error}") + print(f"Error on iteration {sim_idx}: {error}") with open(self._error_file, "a", encoding="utf-8") as f: f.write(inputs_json) raise error diff --git a/tests/unit/simulation/test_monte_carlo_simulation_index.py b/tests/unit/simulation/test_monte_carlo_simulation_index.py new file mode 100644 index 000000000..7d2d6a52a --- /dev/null +++ b/tests/unit/simulation/test_monte_carlo_simulation_index.py @@ -0,0 +1,76 @@ +import json + +import pytest + +from rocketpy.simulation.monte_carlo import MonteCarlo + + +def _indices(analysis): + with open(analysis.output_file, "r", encoding="utf-8") as written: + return sorted(json.loads(line)["index"] for line in written if line.strip()) + + +def _a_study(tmp_path, stem, environment, rocket, flight): + return MonteCarlo( + filename=str(tmp_path / stem), + environment=environment, + rocket=rocket, + flight=flight, + ) + + +@pytest.mark.parametrize("parallel", [False, True]) +def test_a_run_numbers_its_simulations_from_zero( + stochastic_environment, stochastic_calisto, stochastic_flight, tmp_path, parallel +): + # Serial used to write 1, 2, 3 while parallel wrote 0, 1, 2, so the same + # simulation had two names depending on how the run was started. + analysis = _a_study( + tmp_path, + f"study-{parallel}", + stochastic_environment, + stochastic_calisto, + stochastic_flight, + ) + + analysis.simulate( + number_of_simulations=3, + append=False, + parallel=parallel, + n_workers=2 if parallel else None, + ) + + assert _indices(analysis) == [0, 1, 2] + + +def test_both_modes_agree_on_what_a_simulation_is_called( + stochastic_environment, stochastic_calisto, stochastic_flight, tmp_path +): + one = _a_study( + tmp_path, + "serial", + stochastic_environment, + stochastic_calisto, + stochastic_flight, + ) + other = _a_study( + tmp_path, "para", stochastic_environment, stochastic_calisto, stochastic_flight + ) + + one.simulate(number_of_simulations=3, append=False, parallel=False) + other.simulate(number_of_simulations=3, append=False, parallel=True, n_workers=2) + + assert _indices(one) == _indices(other) + + +def test_an_appended_run_carries_on_from_the_last_index( + stochastic_environment, stochastic_calisto, stochastic_flight, tmp_path +): + analysis = _a_study( + tmp_path, "study", stochastic_environment, stochastic_calisto, stochastic_flight + ) + + analysis.simulate(number_of_simulations=2, append=False) + analysis.simulate(number_of_simulations=4, append=True) + + assert _indices(analysis) == [0, 1, 2, 3] From 5fd8408f1eacf63161280a7102231a65e026d433 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Tue, 18 Aug 2026 05:32:34 +0800 Subject: [PATCH 03/15] ENH: derive each simulation's seed from its own index 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 #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 #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> --- rocketpy/simulation/monte_carlo.py | 92 ++++++++-- .../simulation/test_monte_carlo_seeding.py | 157 ++++++++++++++++++ 2 files changed, 238 insertions(+), 11 deletions(-) create mode 100644 tests/unit/simulation/test_monte_carlo_seeding.py diff --git a/rocketpy/simulation/monte_carlo.py b/rocketpy/simulation/monte_carlo.py index cec94fbc3..7c4b86a34 100644 --- a/rocketpy/simulation/monte_carlo.py +++ b/rocketpy/simulation/monte_carlo.py @@ -18,6 +18,7 @@ import os import traceback import warnings +from copy import deepcopy from numbers import Real from pathlib import Path from time import time @@ -31,6 +32,7 @@ from rocketpy.prints.monte_carlo_prints import _MonteCarloPrints from rocketpy.simulation.flight import Flight from rocketpy.tools import ( + _seed_sequence_to_int, generate_monte_carlo_ellipses, generate_monte_carlo_ellipses_coordinates, import_optional_dependency, @@ -44,6 +46,57 @@ _SIMULATION_LOG_SUFFIX = ".txt" +def _root_seed_sequence(random_seed): + """The immutable root a run derives every simulation's seed from. + + A ``SeedSequence`` is rebuilt from its full state rather than used as + given, since ``spawn`` advances a counter the caller still holds. A + ``Generator`` is refused rather than read, because using a consume-on-use + object as an immutable seed cannot mean what it says. + """ + if isinstance(random_seed, np.random.SeedSequence): + return np.random.SeedSequence(**random_seed.state) + if isinstance(random_seed, (np.random.Generator, np.random.BitGenerator)): + raise TypeError( + f"random_seed must be an int, a sequence of non-negative integers, " + f"or a numpy.random.SeedSequence, not a " + f"{type(random_seed).__name__}. Pass the seed the generator was " + f"built from." + ) + return np.random.SeedSequence(random_seed) + + +def _root_state_of(root): + """A root as the four picklable values a worker can rebuild it from. + + Sent to each worker instead of the object, and instead of the list of + children, so a run of a million simulations costs four values. The entropy + is copied because a sequence one is kept by reference all the way from the + caller, who could otherwise still move every child by editing their list. + """ + return ( + deepcopy(root.entropy), + tuple(root.spawn_key), + root.pool_size, + root.n_children_spawned, + ) + + +def _seed_of_simulation(root_state, sim_idx): + """The seed for one simulation index, without spawning the ones before it. + + ``spawn`` derives child ``i`` by appending ``n_children_spawned + i`` to + the parent spawn key, so rebuilding that one child directly reproduces it + and any index can be reached from the four values above alone. + """ + entropy, spawn_key, pool_size, base = root_state + return np.random.SeedSequence( + entropy=entropy, + spawn_key=(*spawn_key, base + sim_idx), + pool_size=pool_size, + ) + + def _refuse_logs_this_run_cannot_write( input_file, output_file, error_file, export_config=None ): @@ -265,6 +318,8 @@ def simulate( append=False, parallel=False, n_workers=None, + *, + random_seed=None, **kwargs, ): """ @@ -317,6 +372,11 @@ def simulate( self._export_config = kwargs self.number_of_simulations = number_of_simulations self._initial_sim_idx = self.num_of_loaded_sims if append else 0 + # Validated here, before __setup_files truncates anything, so an + # unusable seed cannot cost a previous run its results. Kept as four + # picklable values rather than as the object, since a worker rebuilds + # any index from them. + self.__root_state = _root_state_of(_root_seed_sequence(random_seed)) # Before anything is opened: __setup_files truncates for append=False. _refuse_logs_this_run_cannot_write( @@ -422,6 +482,7 @@ def __run_in_serial(self): sim_idx = sim_monitor.increment() - 1 inputs_json, outputs_json = "", "" + self.__seed_this_simulation(sim_idx) flight = self.__run_single_simulation() inputs_json = self.__evaluate_flight_inputs(sim_idx) outputs_json = self.__evaluate_flight_outputs(flight, sim_idx) @@ -473,13 +534,14 @@ def __run_in_parallel(self, n_workers=None): ) processes = [] - seeds = np.random.SeedSequence().spawn(n_workers) - for seed in seeds: + # No seed per worker any more: every simulation takes its own from + # its index, so the workers are interchangeable and how many there + # are does not reach the sampling. + for _ in range(n_workers): sim_producer = multiprocess.Process( target=self.__sim_producer, args=( - seed, sim_monitor, mutex, simulation_error_event, @@ -521,13 +583,11 @@ def __validate_number_of_workers(self, n_workers): raise ValueError("Number of workers must be at least 2 for parallel mode.") return n_workers - def __sim_producer(self, seed, sim_monitor, mutex, error_event): # pylint: disable=too-many-statements + def __sim_producer(self, sim_monitor, mutex, error_event): """Simulation producer to be used in parallel by multiprocessing. Parameters ---------- - seed : int - The seed to set the random number generator. sim_monitor : _SimMonitor The simulation monitor object to keep track of the simulations. mutex : multiprocess.Lock @@ -536,15 +596,11 @@ def __sim_producer(self, seed, sim_monitor, mutex, error_event): # pylint: disa Event signaling an error occurred during the simulation. """ try: - # Ensure Processes generate different random numbers - self.environment._set_stochastic(seed) - self.rocket._set_stochastic(seed) - self.flight._set_stochastic(seed) - while sim_monitor.keep_simulating(): sim_idx = sim_monitor.increment() - 1 inputs_json, outputs_json = "", "" + self.__seed_this_simulation(sim_idx) flight = self.__run_single_simulation() inputs_json = self.__evaluate_flight_inputs(sim_idx) outputs_json = self.__evaluate_flight_outputs(flight, sim_idx) @@ -584,6 +640,20 @@ def __sim_producer(self, seed, sim_monitor, mutex, error_event): # pylint: disa error_event.set() mutex.release() + def __seed_this_simulation(self, sim_idx): + """Reseed the three models from this index's own child of the root. + + Per index rather than per worker, which is what makes a simulation's + inputs the same however the run was split up. The child is split three + ways so the environment, rocket and flight draw independently instead + of sharing one stream. + """ + child = _seed_of_simulation(self.__root_state, sim_idx) + environment, rocket, flight = child.spawn(3) + self.environment._set_stochastic(_seed_sequence_to_int(environment)) + self.rocket._set_stochastic(_seed_sequence_to_int(rocket)) + self.flight._set_stochastic(_seed_sequence_to_int(flight)) + def __run_single_simulation(self): """Runs a single simulation and returns the inputs and outputs. diff --git a/tests/unit/simulation/test_monte_carlo_seeding.py b/tests/unit/simulation/test_monte_carlo_seeding.py new file mode 100644 index 000000000..b820c5152 --- /dev/null +++ b/tests/unit/simulation/test_monte_carlo_seeding.py @@ -0,0 +1,157 @@ +import json +import os + +import numpy as np +import pytest + +from rocketpy.simulation.monte_carlo import ( + MonteCarlo, + _root_seed_sequence, + _root_state_of, + _seed_of_simulation, +) + + +def _sampled_inputs(analysis): + with open(analysis.input_file, "r", encoding="utf-8") as written: + rows = [json.loads(line) for line in written if line.strip()] + return {row["index"]: row for row in rows} + + +def _a_run(tmp_path, stem, models, *, parallel=False, workers=None, seed=None, count=4): + environment, rocket, flight = models + analysis = MonteCarlo( + filename=str(tmp_path / stem), + environment=environment, + rocket=rocket, + flight=flight, + ) + analysis.simulate( + number_of_simulations=count, + append=False, + parallel=parallel, + n_workers=workers, + random_seed=seed, + ) + return analysis + + +@pytest.fixture(name="models") +def _models(stochastic_environment, stochastic_calisto, stochastic_flight): + return stochastic_environment, stochastic_calisto, stochastic_flight + + +# --------------------------------------------------------------- the derivation + + +@pytest.mark.parametrize("seed", [42, None, [1, 2, 3], np.random.SeedSequence(7)]) +def test_a_simulation_gets_the_child_spawn_would_have_given_it(seed): + # The whole point of deriving one index directly: it has to be the same + # child, bit for bit, as spawning every index before it would produce. + root = _root_seed_sequence(seed) + state = _root_state_of(root) + spawned = np.random.SeedSequence(**root.state).spawn(6) + + for index, expected in enumerate(spawned): + assert np.array_equal( + expected.generate_state(4), + _seed_of_simulation(state, index).generate_state(4), + ) + + +def test_deriving_an_index_costs_nothing_for_the_ones_before_it(): + state = _root_state_of(_root_seed_sequence(42)) + + far = _seed_of_simulation(state, 1_000_000) + + assert far.spawn_key == (1_000_000,) + + +def test_two_indices_do_not_share_a_stream(): + state = _root_state_of(_root_seed_sequence(42)) + + first = _seed_of_simulation(state, 0).generate_state(4) + second = _seed_of_simulation(state, 1).generate_state(4) + + assert not np.array_equal(first, second) + + +# ------------------------------------------------------------------- the root + + +def test_a_caller_seed_sequence_is_not_consumed(): + given = np.random.SeedSequence(42) + + _root_seed_sequence(given).spawn(5) + + assert given.n_children_spawned == 0 + + +def test_a_caller_cannot_move_the_run_by_editing_the_list_it_passed(): + # SeedSequence keeps a sequence entropy by reference, so without a copy of + # its own the run would follow whatever the caller did to that list next. + given = [1, 2, 3] + state = _root_state_of(_root_seed_sequence(given)) + + before = _seed_of_simulation(state, 0).generate_state(4) + given[0] = 999 + + assert np.array_equal(_seed_of_simulation(state, 0).generate_state(4), before) + + +@pytest.mark.parametrize("given", [np.random.default_rng(42), np.random.PCG64(42)]) +def test_a_generator_is_refused_rather_than_read(given): + # Using a consume-on-use object as an immutable seed cannot mean what it + # says, so it is refused instead of quietly meaning something else. + with pytest.raises(TypeError, match="random_seed must be"): + _root_seed_sequence(given) + + +def test_an_unusable_seed_costs_the_previous_run_nothing(models, tmp_path): + analysis = _a_run(tmp_path, "study", models, seed=42, count=2) + kept = _sampled_inputs(analysis) + + with pytest.raises(TypeError): + analysis.simulate(2, append=False, random_seed=np.random.default_rng(1)) + + assert _sampled_inputs(analysis) == kept + + +# ------------------------------------------------------------- what a run gives + + +def test_one_seed_gives_one_set_of_inputs(models, tmp_path): + first = _a_run(tmp_path, "first", models, seed=42) + again = _a_run(tmp_path, "again", models, seed=42) + + assert _sampled_inputs(first) == _sampled_inputs(again) + + +def test_another_seed_gives_another_set(models, tmp_path): + # The control. Without this the test above passes on a run that ignores + # the seed entirely. + first = _a_run(tmp_path, "first", models, seed=42) + other = _a_run(tmp_path, "other", models, seed=7) + + assert _sampled_inputs(first) != _sampled_inputs(other) + + +@pytest.mark.skipif(os.cpu_count() < 4, reason="needs four workers to be four") +def test_an_index_gets_the_same_inputs_however_the_run_was_split(models, tmp_path): + # This is the guarantee. Splitting the work differently must not change + # what any one simulation drew. + serial = _a_run(tmp_path, "serial", models, seed=42) + two = _a_run(tmp_path, "two", models, parallel=True, workers=2, seed=42) + four = _a_run(tmp_path, "four", models, parallel=True, workers=4, seed=42) + + assert _sampled_inputs(serial) == _sampled_inputs(two) + assert _sampled_inputs(serial) == _sampled_inputs(four) + + +def test_an_appended_run_carries_the_same_stream_on(models, tmp_path): + whole = _a_run(tmp_path, "whole", models, seed=42, count=4) + + part = _a_run(tmp_path, "part", models, seed=42, count=2) + part.simulate(4, append=True, random_seed=42) + + assert _sampled_inputs(part) == _sampled_inputs(whole) From 34a0a62a5c1d079dd726c23dbc52c3fd4121469c Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Tue, 18 Aug 2026 23:36:50 +0800 Subject: [PATCH 04/15] DOC: say where a Monte Carlo run's seed goes 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> --- docs/user/stochastic.rst | 8 ++++++++ rocketpy/simulation/monte_carlo.py | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/docs/user/stochastic.rst b/docs/user/stochastic.rst index 6e3376236..1fd8c2f56 100644 --- a/docs/user/stochastic.rst +++ b/docs/user/stochastic.rst @@ -289,3 +289,11 @@ better reflecting the inherent uncertainties in rocketry. .. note:: See the ``MonteCarlo`` class documentation for more information on how to run \ Monte Carlo simulations with stochastic objects. + +.. note:: + A whole run is fixed by ``MonteCarlo.simulate(random_seed=...)`` rather than + by seeding these models yourself. Each simulation takes its seed from its + own index, so simulation 7 draws the same inputs whether the run was serial + or split over any number of workers, and appending with the same seed + carries the same stream on. Without it a run draws fresh entropy and + reproduces nothing. diff --git a/rocketpy/simulation/monte_carlo.py b/rocketpy/simulation/monte_carlo.py index 7c4b86a34..44f019119 100644 --- a/rocketpy/simulation/monte_carlo.py +++ b/rocketpy/simulation/monte_carlo.py @@ -339,6 +339,20 @@ def simulate( number of workers will be equal to the number of CPUs available. A minimum of 2 workers is required for parallel mode. Default is None. + random_seed : int, sequence of int or numpy.random.SeedSequence, optional + Fixes what every simulation draws. Simulation ``i`` takes the same + inputs whichever way the run was split up, so serial and parallel + results agree and the number of workers does not reach the + sampling. Keyword-only. Default is None, which draws fresh entropy + and reproduces nothing. + + 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 that a later + append was given the same one; that is #1075. + + A ``Generator`` or ``BitGenerator`` is refused rather than read. + Pass the seed it was built from. kwargs : dict Custom arguments for simulation export of the ``inputs`` file. Options are: From 7b8549950a10ed5f8e3e6c93ea77c4dbdf01fb54 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Tue, 25 Aug 2026 22:28:50 +0800 Subject: [PATCH 05/15] TST: compare what a simulation drew, not which process wrote it 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> --- .../simulation/test_monte_carlo_seeding.py | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/unit/simulation/test_monte_carlo_seeding.py b/tests/unit/simulation/test_monte_carlo_seeding.py index b820c5152..b83818023 100644 --- a/tests/unit/simulation/test_monte_carlo_seeding.py +++ b/tests/unit/simulation/test_monte_carlo_seeding.py @@ -12,10 +12,30 @@ ) +def _without_object_identity(value): + """The same record with every ``hash`` dropped, at any depth. + + ``RocketPyEncoder`` records ``hash(obj)`` beside a serialized ``Function``, + and that is the object's identity in the process that wrote it, not + anything that was drawn. It agrees between two runs that share memory and + differs between two that do not, which is a property of the start method + rather than of the seed. + """ + if isinstance(value, dict): + return { + key: _without_object_identity(item) + for key, item in value.items() + if key != "hash" + } + if isinstance(value, list): + return [_without_object_identity(item) for item in value] + return value + + def _sampled_inputs(analysis): with open(analysis.input_file, "r", encoding="utf-8") as written: rows = [json.loads(line) for line in written if line.strip()] - return {row["index"]: row for row in rows} + return {row["index"]: _without_object_identity(row) for row in rows} def _a_run(tmp_path, stem, models, *, parallel=False, workers=None, seed=None, count=4): From f9a10fa2829576f878ec1ef9b44c8eb175bbbde1 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Wed, 26 Aug 2026 00:13:36 +0800 Subject: [PATCH 06/15] TST: compare what a simulation drew, not what wrote the row down 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> --- .../simulation/test_monte_carlo_seeding.py | 52 +++++++++++++++---- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/tests/unit/simulation/test_monte_carlo_seeding.py b/tests/unit/simulation/test_monte_carlo_seeding.py index b83818023..a24829789 100644 --- a/tests/unit/simulation/test_monte_carlo_seeding.py +++ b/tests/unit/simulation/test_monte_carlo_seeding.py @@ -1,6 +1,7 @@ import json import os +import multiprocess import numpy as np import pytest @@ -12,30 +13,32 @@ ) -def _without_object_identity(value): - """The same record with every ``hash`` dropped, at any depth. +def _what_was_drawn(value): + """The same record without the parts that say which process wrote it. - ``RocketPyEncoder`` records ``hash(obj)`` beside a serialized ``Function``, - and that is the object's identity in the process that wrote it, not - anything that was drawn. It agrees between two runs that share memory and - differs between two that do not, which is a property of the start method - rather than of the seed. + Two of the fields a serialized ``Function`` carries are properties of the + writer rather than of 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 under ``spawn``: measured on one drag curve, the parent + and a forked child agree and a spawned child does not, for a value the + fixture does not vary at all. Everything a run actually draws is numeric + and stays. """ if isinstance(value, dict): return { - key: _without_object_identity(item) + key: _what_was_drawn(item) for key, item in value.items() - if key != "hash" + if key != "hash" and not (key == "source" and isinstance(item, str)) } if isinstance(value, list): - return [_without_object_identity(item) for item in value] + return [_what_was_drawn(item) for item in value] return value def _sampled_inputs(analysis): with open(analysis.input_file, "r", encoding="utf-8") as written: rows = [json.loads(line) for line in written if line.strip()] - return {row["index"]: _without_object_identity(row) for row in rows} + return {row["index"]: _what_was_drawn(row) for row in rows} def _a_run(tmp_path, stem, models, *, parallel=False, workers=None, seed=None, count=4): @@ -175,3 +178,30 @@ def test_an_appended_run_carries_the_same_stream_on(models, tmp_path): part.simulate(4, append=True, random_seed=42) assert _sampled_inputs(part) == _sampled_inputs(whole) + + +@pytest.fixture(name="spawned_workers") +def _spawned_workers(): + """Start workers the way Windows does, wherever the test happens to run.""" + was = multiprocess.get_start_method() + multiprocess.set_start_method("spawn", force=True) + yield + multiprocess.set_start_method(was, force=True) + + +@pytest.mark.usefixtures("spawned_workers") +@pytest.mark.skipif(os.cpu_count() < 4, reason="needs four workers to be four") +def test_an_index_keeps_its_inputs_when_the_workers_are_spawned(models, tmp_path): + """The same guarantee on the start method Windows uses. + + A spawned child rebuilds the models by unpickling rather than inheriting + them, so this is a different question from the one above and was worth + asking separately: the first version of these tests compared serialized + callables, which differ between processes for a value nothing varies. + """ + serial = _a_run(tmp_path, "serial", models, seed=42) + two = _a_run(tmp_path, "two", models, parallel=True, workers=2, seed=42) + four = _a_run(tmp_path, "four", models, parallel=True, workers=4, seed=42) + + assert _sampled_inputs(serial) == _sampled_inputs(two) + assert _sampled_inputs(serial) == _sampled_inputs(four) From fac72c2c20d6247351a1aa321058137555de0719 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:40:05 +0800 Subject: [PATCH 07/15] TST: cover the failure message and the loop a worker runs 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> --- .../simulation/test_monte_carlo_seeding.py | 45 +++++++++++++++++++ .../test_monte_carlo_simulation_index.py | 37 +++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/tests/unit/simulation/test_monte_carlo_seeding.py b/tests/unit/simulation/test_monte_carlo_seeding.py index a24829789..fd8d360fa 100644 --- a/tests/unit/simulation/test_monte_carlo_seeding.py +++ b/tests/unit/simulation/test_monte_carlo_seeding.py @@ -1,5 +1,6 @@ import json import os +from time import time import multiprocess import numpy as np @@ -10,6 +11,7 @@ _root_seed_sequence, _root_state_of, _seed_of_simulation, + _SimMonitor, ) @@ -205,3 +207,46 @@ def test_an_index_keeps_its_inputs_when_the_workers_are_spawned(models, tmp_path assert _sampled_inputs(serial) == _sampled_inputs(two) assert _sampled_inputs(serial) == _sampled_inputs(four) + + +class _ALockNobodyContends: + """The manager's mutex, with only this process to hand it to.""" + + def acquire(self): + pass + + def release(self): + pass + + +class _AnEventNobodySet: + """The failure event, which nothing in a clean run reaches for.""" + + def is_set(self): + return False + + def set(self): + raise AssertionError("the producer reported a failure") + + +def test_a_worker_loop_draws_the_study_serial_draws(models, tmp_path): + """The loop a worker runs, driven here, produces the run serial produces.""" + # Every other check of this starts a child process, where the same code + # runs and nothing in the test can watch it. + serial = _a_run(tmp_path, "serial", models, seed=42, count=4) + environment, rocket, flight = models + worker = MonteCarlo( + filename=str(tmp_path / "worker"), + environment=environment, + rocket=rocket, + flight=flight, + ) + worker.simulate(number_of_simulations=0, append=False, random_seed=42) + + worker._MonteCarlo__sim_producer( + _SimMonitor(initial_count=0, n_simulations=4, start_time=time()), + _ALockNobodyContends(), + _AnEventNobodySet(), + ) + + assert _sampled_inputs(worker) == _sampled_inputs(serial) diff --git a/tests/unit/simulation/test_monte_carlo_simulation_index.py b/tests/unit/simulation/test_monte_carlo_simulation_index.py index 7d2d6a52a..c570bbea1 100644 --- a/tests/unit/simulation/test_monte_carlo_simulation_index.py +++ b/tests/unit/simulation/test_monte_carlo_simulation_index.py @@ -74,3 +74,40 @@ def test_an_appended_run_carries_on_from_the_last_index( analysis.simulate(number_of_simulations=4, append=True) assert _indices(analysis) == [0, 1, 2, 3] + + +def test_a_serial_failure_names_the_simulation_the_way_parallel_would( + stochastic_environment, + stochastic_calisto, + stochastic_flight, + tmp_path, + monkeypatch, + capsys, +): + """A run that fails reports the index the other mode would have used.""" + # The message is the only place the numbering reaches whoever ran it, so + # it can disagree with the logs without anything else noticing. + analysis = _a_study( + tmp_path, + "failing", + stochastic_environment, + stochastic_calisto, + stochastic_flight, + ) + attempts = [] + ran = MonteCarlo._MonteCarlo__run_single_simulation + + def fails_on_the_second(self): + attempts.append(None) + if len(attempts) == 2: + raise RuntimeError("the flight would not run") + return ran(self) + + monkeypatch.setattr( + MonteCarlo, "_MonteCarlo__run_single_simulation", fails_on_the_second + ) + + with pytest.raises(RuntimeError, match="the flight would not run"): + analysis.simulate(number_of_simulations=3, append=False) + + assert "Error on iteration 1:" in capsys.readouterr().out From 765edb25e74e6e17f7ea0b41af0f69c52e498662 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:40:05 +0800 Subject: [PATCH 08/15] DOC: give this branch its changelog line The workflow that would have written it has not run since #1112, which is what #1173 is about. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28f601312..1472a7a40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Attention: The newest changes should be on top --> ### Added +- ENH: Reproducible Monte Carlo through per-simulation-index seeding [#1054](https://github.com/RocketPy-Team/RocketPy/pull/1054) [#1053](https://github.com/RocketPy-Team/RocketPy/issues/1053) - ENH: Support fixed-time parachute deployment triggers [#1133](https://github.com/RocketPy-Team/RocketPy/pull/1133) [#437](https://github.com/RocketPy-Team/RocketPy/issues/437) - DOC: Add SIL parachute ejection integration example [#1131](https://github.com/RocketPy-Team/RocketPy/pull/1131) [#524](https://github.com/RocketPy-Team/RocketPy/issues/524) - ENH: List NOAA atmosphere datasets and fetch latest [#1136](https://github.com/RocketPy-Team/RocketPy/pull/1136) [#660](https://github.com/RocketPy-Team/RocketPy/issues/660) From 3aada2cd02b97c9973b66474684ae23fc94e1ae7 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Thu, 3 Sep 2026 17:36:29 +0800 Subject: [PATCH 09/15] BUG: claim each simulation index in one operation keep_simulating() and increment() were two calls on a manager proxy, so two workers could both see the last slot free and then claim an index each, one of them past the end. claim_next_index() does both under a lock in the manager's own process, and the two methods it replaces are gone so the pair cannot be written again. The index also moves after the data collectors, so one named "index" cannot take the number the row's seed was derived from. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/simulation/monte_carlo.py | 35 +++++++----- .../test_monte_carlo_simulation_index.py | 57 ++++++++++++++++++- 2 files changed, 77 insertions(+), 15 deletions(-) diff --git a/rocketpy/simulation/monte_carlo.py b/rocketpy/simulation/monte_carlo.py index 44f019119..fbfa4868a 100644 --- a/rocketpy/simulation/monte_carlo.py +++ b/rocketpy/simulation/monte_carlo.py @@ -16,6 +16,7 @@ import csv import json import os +import threading import traceback import warnings from copy import deepcopy @@ -489,11 +490,10 @@ def __run_in_serial(self): ) sim_idx = sim_monitor.count try: - while sim_monitor.keep_simulating(): - # Counted from zero, as the parallel path already does. The two - # named the same simulation differently: three of them wrote - # 1, 2, 3 here and 0, 1, 2 there. - sim_idx = sim_monitor.increment() - 1 + # Counted from zero, as the parallel path already does: the two + # used to name the same simulation 1, 2, 3 and 0, 1, 2. + while (claimed := sim_monitor.claim_next_index()) is not None: + sim_idx = claimed inputs_json, outputs_json = "", "" self.__seed_this_simulation(sim_idx) @@ -610,8 +610,7 @@ def __sim_producer(self, sim_monitor, mutex, error_event): Event signaling an error occurred during the simulation. """ try: - while sim_monitor.keep_simulating(): - sim_idx = sim_monitor.increment() - 1 + while (sim_idx := sim_monitor.claim_next_index()) is not None: inputs_json, outputs_json = "", "" self.__seed_this_simulation(sim_idx) @@ -911,8 +910,6 @@ def __evaluate_flight_outputs(self, flight, sim_idx): export_item: getattr(flight, export_item) for export_item in self.export_list } - outputs_dict["index"] = sim_idx - if self.data_collector is not None: additional_exports = {} for key, callback in self.data_collector.items(): @@ -924,6 +921,9 @@ def __evaluate_flight_outputs(self, flight, sim_idx): ) from e outputs_dict = outputs_dict | additional_exports + # After the collectors, so one cannot take the row's own number. + outputs_dict["index"] = sim_idx + return ( json.dumps(outputs_dict, cls=RocketPyEncoder, **self._export_config) + "\n" ) @@ -1899,13 +1899,20 @@ def __init__(self, initial_count, n_simulations, start_time): self.n_simulations = n_simulations self.start_time = start_time self.completed_count = 0 + self._claim = threading.Lock() # proxy calls run in the manager - def keep_simulating(self): - return self.count < self.n_simulations + def claim_next_index(self): + """The next index to run, or None once every one has been claimed. - def increment(self): - self.count += 1 - return self.count + One call, because a separate check and increment let two workers both + see the last slot free and then claim an index each past the end. + """ + with self._claim: + if self.count >= self.n_simulations: + return None + claimed = self.count + self.count += 1 + return claimed def print_update_status(self): """Prints a message on the same line as the previous one and replaces diff --git a/tests/unit/simulation/test_monte_carlo_simulation_index.py b/tests/unit/simulation/test_monte_carlo_simulation_index.py index c570bbea1..1c22c3c5b 100644 --- a/tests/unit/simulation/test_monte_carlo_simulation_index.py +++ b/tests/unit/simulation/test_monte_carlo_simulation_index.py @@ -1,8 +1,10 @@ import json +import threading +from time import time import pytest -from rocketpy.simulation.monte_carlo import MonteCarlo +from rocketpy.simulation.monte_carlo import MonteCarlo, _SimMonitor def _indices(analysis): @@ -111,3 +113,56 @@ def fails_on_the_second(self): analysis.simulate(number_of_simulations=3, append=False) assert "Error on iteration 1:" in capsys.readouterr().out + + +def test_claim_next_index_hands_out_each_index_once(): + """Claimers racing each other get range(n) between them and nothing past it.""" + n_simulations, n_claimers = 200, 8 + monitor = _SimMonitor(0, n_simulations, time()) + ready = threading.Barrier(n_claimers) + guard = threading.Lock() + claimed = [] + + def claim_until_empty(): + ready.wait() + mine = [] + while (index := monitor.claim_next_index()) is not None: + mine.append(index) + with guard: + claimed.extend(mine) + + workers = [threading.Thread(target=claim_until_empty) for _ in range(n_claimers)] + for worker in workers: + worker.start() + for worker in workers: + worker.join() + + assert len(claimed) == n_simulations + assert len(set(claimed)) == n_simulations + assert sorted(claimed) == list(range(n_simulations)) + assert max(claimed) < n_simulations + + +def test_claim_next_index_is_empty_once_the_target_is_reached(): + """The control: a checkpoint that already holds the target claims nothing.""" + monitor = _SimMonitor(3, 3, time()) + + assert monitor.claim_next_index() is None + assert monitor.count == 3 + + +def test_a_collector_cannot_take_over_the_simulation_index( + stochastic_environment, stochastic_calisto, stochastic_flight, tmp_path +): + """The index names the seed a row was drawn with, so nothing else sets it.""" + # Set past the constructor, so this pins the row and not the validation. + analysis = _a_study( + tmp_path, "study", stochastic_environment, stochastic_calisto, stochastic_flight + ) + analysis.export_list = [] + analysis._export_config = {} + analysis.data_collector = {"index": lambda _flight: 999} + + row = json.loads(analysis._MonteCarlo__evaluate_flight_outputs(None, 7)) + + assert row["index"] == 7 From 97f3b45c1221aaddb9cbaf328844e9ddf8f17475 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Fri, 4 Sep 2026 00:06:44 +0800 Subject: [PATCH 10/15] DOC: say what an index does not fix A stochastic input reads its nominal from the object it wraps, and a run moves that as it goes, so two runs of the same index can draw the same inputs and still fly differently. Measured over four simulations with a wind factor: the recorded inputs match serial to two workers for every index, and three of the four apogees do not. What this branch fixes is the inputs an index draws, which is what the note says now. The index test also stubs the root state, which a later branch writes into the same row. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- docs/user/stochastic.rst | 4 ++++ tests/unit/simulation/test_monte_carlo_simulation_index.py | 2 ++ 2 files changed, 6 insertions(+) diff --git a/docs/user/stochastic.rst b/docs/user/stochastic.rst index 1fd8c2f56..f8b31db14 100644 --- a/docs/user/stochastic.rst +++ b/docs/user/stochastic.rst @@ -297,3 +297,7 @@ better reflecting the inherent uncertainties in rocketry. or split over any number of workers, and appending with the same seed carries the same stream on. Without it a run draws fresh entropy and reproduces nothing. + + What an index fixes is the inputs it draws. A stochastic input reads its + nominal from the object it wraps, and a run moves that as it goes, so the + flight itself can still differ by which worker took the index. diff --git a/tests/unit/simulation/test_monte_carlo_simulation_index.py b/tests/unit/simulation/test_monte_carlo_simulation_index.py index 1c22c3c5b..4ac75b67f 100644 --- a/tests/unit/simulation/test_monte_carlo_simulation_index.py +++ b/tests/unit/simulation/test_monte_carlo_simulation_index.py @@ -161,6 +161,8 @@ def test_a_collector_cannot_take_over_the_simulation_index( ) analysis.export_list = [] analysis._export_config = {} + # The row also carries which root drew it, which simulate() would have set. + analysis._MonteCarlo__root_state = (42, (), 4, 0) analysis.data_collector = {"index": lambda _flight: 999} row = json.loads(analysis._MonteCarlo__evaluate_flight_outputs(None, 7)) From c7bffc38c7d0a4185c35539322b1c6d6015985eb Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Wed, 26 Aug 2026 01:02:49 +0800 Subject: [PATCH 11/15] ENH: let an append carry on from the root the rows were drawn with 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 #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> --- rocketpy/simulation/monte_carlo.py | 102 +++++++++++++++- tests/unit/simulation/test_append_lineage.py | 121 +++++++++++++++++++ 2 files changed, 222 insertions(+), 1 deletion(-) create mode 100644 tests/unit/simulation/test_append_lineage.py diff --git a/rocketpy/simulation/monte_carlo.py b/rocketpy/simulation/monte_carlo.py index fbfa4868a..ad9603f0e 100644 --- a/rocketpy/simulation/monte_carlo.py +++ b/rocketpy/simulation/monte_carlo.py @@ -46,6 +46,12 @@ # this is the only format it can both resume from and overwrite safely. _SIMULATION_LOG_SUFFIX = ".txt" +# Which root drew a row. An append reads it to continue the same stream. +_SIMULATION_ROOT_KEY = "run_root" + +# Told apart from a row that carries ``None``, which no run writes. +_NOTHING_READ_YET = object() + def _root_seed_sequence(random_seed): """The immutable root a run derives every simulation's seed from. @@ -67,6 +73,68 @@ def _root_seed_sequence(random_seed): return np.random.SeedSequence(random_seed) +def _jsonable_entropy(entropy): + """``SeedSequence`` entropy as something ``json`` will take. + + It may be an int, a sequence or an ndarray; only the first survives. + """ + if entropy is None or isinstance(entropy, (int, np.integer)): + return None if entropy is None else int(entropy) + return [int(part) for part in np.asarray(entropy).ravel()] + + +def _root_written_into_a_row(root_state): + """The run's root as one JSON value, carried by every input row. + + In the rows because a file beside a log cannot be shown to belong to it. + """ + entropy, spawn_key, pool_size, base = root_state + return { + "entropy": _jsonable_entropy(entropy), + "spawn_key": [int(key) for key in spawn_key], + "pool_size": int(pool_size), + "n_children_spawned": int(base), + } + + +def _root_a_log_was_written_with(path): + """The root every row of a log agrees on, or ``None`` if it holds none. + + ``None`` means the log holds no rows, and nothing else does. Rows that + carry no root are refused instead: they cannot be shown to be one study, + and reading them as an empty log would start a second one in the file. + A log whose rows disagree is refused for the same reason. + """ + first = _NOTHING_READ_YET + with open(path, "r", encoding="utf-8") as recorded: + for line in recorded: + if not line.strip(): + continue + try: + row = json.loads(line) + except ValueError as error: + raise ValueError( + f"cannot continue {path}: a row cannot be read, so what " + f"produced it cannot be established." + ) from error + if _SIMULATION_ROOT_KEY not in row: + raise ValueError( + f"cannot continue {path}: a row does not say which root " + f"drew it, which is how a study written before this " + f"release looks. Start a new one rather than continuing " + f"one whose rows cannot be checked." + ) + root = row[_SIMULATION_ROOT_KEY] + if first is _NOTHING_READ_YET: + first = root + elif root != first: + raise ValueError( + f"cannot continue {path}: its rows were not all drawn " + f"from one root, so it holds more than one study." + ) + return None if first is _NOTHING_READ_YET else first + + def _root_state_of(root): """A root as the four picklable values a worker can rebuild it from. @@ -392,12 +460,16 @@ def simulate( # picklable values rather than as the object, since a worker rebuilds # any index from them. self.__root_state = _root_state_of(_root_seed_sequence(random_seed)) - # Before anything is opened: __setup_files truncates for append=False. _refuse_logs_this_run_cannot_write( self.input_file, self.output_file, self.error_file, kwargs ) + # After that one, which says plainly that a .csv cannot be a working + # log. Reaching this first would report it as a row that cannot be read. + if append: + self.__continue_the_root_the_rows_carry(random_seed) + print("Starting Monte Carlo analysis") self.__setup_files(append) @@ -653,6 +725,33 @@ def __sim_producer(self, sim_monitor, mutex, error_event): error_event.set() mutex.release() + def __continue_the_root_the_rows_carry(self, random_seed): + """Take the root from the rows being appended to, or refuse to. + + Without this an append draws a second root into one file and nothing + afterwards can tell which simulation came from which. Reading it back + also means a fresh object can continue a study, which is the ordinary + way of resuming one. + """ + recorded = _root_a_log_was_written_with(self.input_file) + if recorded is None: + return + if random_seed is None: + self.__root_state = ( + recorded["entropy"], + tuple(recorded["spawn_key"]), + recorded["pool_size"], + recorded["n_children_spawned"], + ) + return + if _root_written_into_a_row(self.__root_state) != recorded: + raise ValueError( + f"cannot append to {self.input_file}: its rows were drawn from " + f"a different root than random_seed gives. Continuing would put " + f"two studies in one file. Pass the seed the run started with, " + f"or leave random_seed out to carry on from the rows." + ) + def __seed_this_simulation(self, sim_idx): """Reseed the three models from this index's own child of the root. @@ -887,6 +986,7 @@ def __evaluate_flight_inputs(self, sim_idx): for item in d.items() ) inputs_dict["index"] = sim_idx + inputs_dict[_SIMULATION_ROOT_KEY] = _root_written_into_a_row(self.__root_state) return ( json.dumps(inputs_dict, cls=RocketPyEncoder, **self._export_config) + "\n" ) diff --git a/tests/unit/simulation/test_append_lineage.py b/tests/unit/simulation/test_append_lineage.py new file mode 100644 index 000000000..dc19c7f96 --- /dev/null +++ b/tests/unit/simulation/test_append_lineage.py @@ -0,0 +1,121 @@ +import json + +import pytest + +from rocketpy.simulation.monte_carlo import MonteCarlo, _SIMULATION_ROOT_KEY + + +def _study(tmp_path, stem, models): + environment, rocket, flight = models + return MonteCarlo( + filename=str(tmp_path / stem), + environment=environment, + rocket=rocket, + flight=flight, + ) + + +def _drawn(analysis): + with open(analysis.input_file, "r", encoding="utf-8") as written: + rows = [json.loads(line) for line in written if line.strip()] + return {row["index"]: row.get("mass") for row in rows} + + +@pytest.fixture(name="models") +def _models(stochastic_environment, stochastic_calisto, stochastic_flight): + return stochastic_environment, stochastic_calisto, stochastic_flight + + +def test_a_row_says_which_root_drew_it(models, tmp_path): + """Every input row carries the root, so the log describes itself.""" + analysis = _study(tmp_path, "study", models) + + analysis.simulate(2, append=False, random_seed=42) + + with open(analysis.input_file, "r", encoding="utf-8") as written: + rows = [json.loads(line) for line in written if line.strip()] + assert all(row[_SIMULATION_ROOT_KEY]["entropy"] == 42 for row in rows) + + +def test_an_append_with_no_seed_carries_on_from_the_rows(models, tmp_path): + """The ordinary resume: no seed given, the stream continues anyway.""" + whole = _study(tmp_path, "whole", models) + whole.simulate(4, append=False, random_seed=42) + + part = _study(tmp_path, "part", models) + part.simulate(2, append=False, random_seed=42) + part.simulate(4, append=True) + + assert _drawn(part) == _drawn(whole) + + +def test_a_fresh_object_can_continue_a_study(models, tmp_path): + """A notebook restart is a new object over the same files.""" + first = _study(tmp_path, "study", models) + first.simulate(2, append=False, random_seed=42) + + second = _study(tmp_path, "study", models) + second.simulate(4, append=True) + + assert sorted(_drawn(second)) == [0, 1, 2, 3] + + +def test_appending_with_another_seed_is_refused(models, tmp_path): + """Two roots in one file is the thing this exists to prevent.""" + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + + with pytest.raises(ValueError, match="different root"): + analysis.simulate(4, append=True, random_seed=7) + + +def test_appending_with_the_same_seed_is_allowed(models, tmp_path): + """The control. Saying the seed again is not a mismatch.""" + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + + analysis.simulate(4, append=True, random_seed=42) + + assert sorted(_drawn(analysis)) == [0, 1, 2, 3] + + +def test_a_log_holding_two_studies_is_refused(models, tmp_path): + """Rows that disagree are two studies, and neither is safe to continue.""" + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + with open(analysis.input_file, "r", encoding="utf-8") as written: + rows = [json.loads(line) for line in written if line.strip()] + rows[1][_SIMULATION_ROOT_KEY]["entropy"] = 999 + with open(analysis.input_file, "w", encoding="utf-8") as rewritten: + for row in rows: + rewritten.write(json.dumps(row) + "\n") + + with pytest.raises(ValueError, match="more than one study"): + analysis.simulate(4, append=True) + + +def test_a_log_whose_rows_carry_no_root_is_refused(models, tmp_path): + """A study from before this release cannot be shown to be one study.""" + # Measured before this was refused: the append read the log as empty, + # started a root of its own, and left two lineages in the one file. + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + with open(analysis.input_file, "r", encoding="utf-8") as written: + rows = [json.loads(line) for line in written if line.strip()] + for row in rows: + row.pop(_SIMULATION_ROOT_KEY) + with open(analysis.input_file, "w", encoding="utf-8") as rewritten: + for row in rows: + rewritten.write(json.dumps(row) + "\n") + + with pytest.raises(ValueError, match="does not say which root"): + analysis.simulate(4, append=True) + + +def test_an_empty_log_is_not_a_log_that_cannot_be_checked(models, tmp_path): + """The control. Nothing recorded yet is a fresh start, not a refusal.""" + analysis = _study(tmp_path, "study", models) + + analysis.simulate(2, append=True, random_seed=42) + + assert sorted(_drawn(analysis)) == [0, 1] From 61dcb0a15127fb39cb2b96da6d238821de460d6e Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:13:54 +0800 Subject: [PATCH 12/15] TST: cover the seeds and rows an append cannot carry on from 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> --- tests/unit/simulation/test_append_lineage.py | 49 ++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/unit/simulation/test_append_lineage.py b/tests/unit/simulation/test_append_lineage.py index dc19c7f96..aef91add3 100644 --- a/tests/unit/simulation/test_append_lineage.py +++ b/tests/unit/simulation/test_append_lineage.py @@ -119,3 +119,52 @@ def test_an_empty_log_is_not_a_log_that_cannot_be_checked(models, tmp_path): analysis.simulate(2, append=True, random_seed=42) assert sorted(_drawn(analysis)) == [0, 1] + + +def test_a_seed_given_as_a_sequence_is_recorded_as_one(models, tmp_path): + """A sequence is a documented seed, so the row has to carry it whole.""" + analysis = _study(tmp_path, "study", models) + + analysis.simulate(2, append=False, random_seed=[1, 2, 3]) + + with open(analysis.input_file, "r", encoding="utf-8") as written: + rows = [json.loads(line) for line in written if line.strip()] + assert all(row[_SIMULATION_ROOT_KEY]["entropy"] == [1, 2, 3] for row in rows) + + +def test_a_sequence_seed_still_continues_the_same_study(models, tmp_path): + """And reads back, since a root that cannot be compared refuses the append.""" + whole = _study(tmp_path, "whole", models) + whole.simulate(4, append=False, random_seed=[1, 2, 3]) + + part = _study(tmp_path, "part", models) + part.simulate(2, append=False, random_seed=[1, 2, 3]) + part.simulate(4, append=True) + + assert _drawn(part) == _drawn(whole) + + +def test_blank_lines_between_rows_do_not_hide_the_root(models, tmp_path): + """A gap in the file is not a row, and not a study without a root either.""" + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + with open(analysis.input_file, "r", encoding="utf-8") as written: + rows = written.read().splitlines() + with open(analysis.input_file, "w", encoding="utf-8") as spaced: + for row in rows: + spaced.write(row + "\n\n") + + analysis.simulate(4, append=True) + + assert sorted(_drawn(analysis)) == [0, 1, 2, 3] + + +def test_a_row_that_cannot_be_read_is_refused(models, tmp_path): + """A row that will not parse leaves nothing to check the root against.""" + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + with open(analysis.input_file, "a", encoding="utf-8") as damaged: + damaged.write("{ this was cut off\n") + + with pytest.raises(ValueError, match="cannot be read"): + analysis.simulate(4, append=True) From eee7debf757ef8c8e7f9a7c6bfc0d23e2229353e Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Wed, 26 Aug 2026 08:13:54 +0800 Subject: [PATCH 13/15] DOC: say what an append carries where it said otherwise The seed docstring still pointed at #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> --- CHANGELOG.md | 1 + docs/user/stochastic.rst | 7 ++++--- rocketpy/simulation/monte_carlo.py | 7 +++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1472a7a40..c50a73d59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Attention: The newest changes should be on top --> ### Added +- ENH: Continue a Monte Carlo study from the root its rows were drawn with [#1187](https://github.com/RocketPy-Team/RocketPy/pull/1187) [#1075](https://github.com/RocketPy-Team/RocketPy/issues/1075) - ENH: Reproducible Monte Carlo through per-simulation-index seeding [#1054](https://github.com/RocketPy-Team/RocketPy/pull/1054) [#1053](https://github.com/RocketPy-Team/RocketPy/issues/1053) - ENH: Support fixed-time parachute deployment triggers [#1133](https://github.com/RocketPy-Team/RocketPy/pull/1133) [#437](https://github.com/RocketPy-Team/RocketPy/issues/437) - DOC: Add SIL parachute ejection integration example [#1131](https://github.com/RocketPy-Team/RocketPy/pull/1131) [#524](https://github.com/RocketPy-Team/RocketPy/issues/524) diff --git a/docs/user/stochastic.rst b/docs/user/stochastic.rst index f8b31db14..907f980ae 100644 --- a/docs/user/stochastic.rst +++ b/docs/user/stochastic.rst @@ -294,9 +294,10 @@ better reflecting the inherent uncertainties in rocketry. A whole run is fixed by ``MonteCarlo.simulate(random_seed=...)`` rather than by seeding these models yourself. Each simulation takes its seed from its own index, so simulation 7 draws the same inputs whether the run was serial - or split over any number of workers, and appending with the same seed - carries the same stream on. Without it a run draws fresh entropy and - reproduces nothing. + or split over any number of workers. Every input row records the root it + came from, so appending carries that study on whether or not the seed is + given again, and a different one is refused rather than mixed in. Without + a seed a run draws fresh entropy and reproduces nothing. What an index fixes is the inputs it draws. A stochastic input reads its nominal from the object it wraps, and a run moves that as it goes, so the diff --git a/rocketpy/simulation/monte_carlo.py b/rocketpy/simulation/monte_carlo.py index ad9603f0e..910ca3fd3 100644 --- a/rocketpy/simulation/monte_carlo.py +++ b/rocketpy/simulation/monte_carlo.py @@ -415,10 +415,9 @@ def simulate( sampling. Keyword-only. Default is None, which draws fresh entropy and reproduces nothing. - 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 that a later - append was given the same one; that is #1075. + Every input row carries the root it was drawn from, so an append + carries on from the study already in the file whether or not the + seed is given again. A different one is refused, not mixed in. A ``Generator`` or ``BitGenerator`` is refused rather than read. Pass the seed it was built from. From b31bb7868524544921561cf1b9edc4ee0e5eb904 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Thu, 3 Sep 2026 17:36:29 +0800 Subject: [PATCH 14/15] BUG: refuse a checkpoint whose halves do not name one study A row carrying run_root: null read as an empty log, so an append started a second root behind it, which is what this check exists to prevent. Rows agreeing on a root also did not make it one a stream can be rebuilt from: SeedSequence(entropy=None) draws fresh entropy every resume. The output rows now carry the root's digest, so an output log from another study with the same indices is refused rather than continued, and both logs are checked against each other before the run decides where to carry on from. A digest rather than the root itself, which would cost 122 bytes a row on a study the input log already records it for. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/simulation/monte_carlo.py | 106 ++++++++++++++-- tests/unit/simulation/test_append_lineage.py | 123 ++++++++++++++++++- 2 files changed, 216 insertions(+), 13 deletions(-) diff --git a/rocketpy/simulation/monte_carlo.py b/rocketpy/simulation/monte_carlo.py index 910ca3fd3..6feba2e79 100644 --- a/rocketpy/simulation/monte_carlo.py +++ b/rocketpy/simulation/monte_carlo.py @@ -14,6 +14,7 @@ """ import csv +import hashlib import json import os import threading @@ -97,8 +98,75 @@ def _root_written_into_a_row(root_state): } -def _root_a_log_was_written_with(path): - """The root every row of a log agrees on, or ``None`` if it holds none. +_ROOT_FIELDS = frozenset(("entropy", "spawn_key", "pool_size", "n_children_spawned")) + + +def _root_digest(root): + """A short stable name for a root, for rows that only have to match one. + + Carried by the output rows instead of the root itself, which would cost a + hundred bytes a row on a study the input log already records it for. + """ + canonical = json.dumps(root, sort_keys=True, separators=(",", ":")) + return hashlib.sha256(canonical.encode("utf-8")).hexdigest()[:16] + + +def _a_whole_number(value): + """A non-negative int, and not a bool standing in for one.""" + return isinstance(value, int) and not isinstance(value, bool) and value >= 0 + + +def _whole_numbers(value, may_be_empty=False): + """A list of non-negative ints, empty only where that is a valid one.""" + return ( + isinstance(value, list) + and (may_be_empty or len(value) > 0) + and all(_a_whole_number(part) for part in value) + ) + + +def _root_state_a_row_records(root, path): + """The four values a root is rebuilt from, refused unless all are usable. + + Agreeing rows show the study is one study, not that what they agree on can + be resumed: ``SeedSequence(entropy=None)`` draws fresh entropy every time. + """ + entropy = root.get("entropy") if isinstance(root, dict) else None + usable = ( + isinstance(root, dict) + and set(root) == _ROOT_FIELDS + and (_a_whole_number(entropy) or _whole_numbers(entropy)) + and _whole_numbers(root.get("spawn_key"), may_be_empty=True) + and _a_whole_number(root.get("pool_size")) + and _a_whole_number(root.get("n_children_spawned")) + ) + if usable: + state = ( + entropy, + tuple(root["spawn_key"]), + root["pool_size"], + root["n_children_spawned"], + ) + try: + # Drawing one child is what proves the pool size numpy will take. + _seed_of_simulation(state, 0) + return state + except ValueError: + pass + raise ValueError( + f"cannot continue {path}: its rows record a root that no stream " + f"can be rebuilt from, so what they were drawn with is unknown." + ) + + +def _rows_a_log_holds(path): + """How many simulations a log records, blank lines aside.""" + with open(path, "r", encoding="utf-8") as recorded: + return sum(1 for line in recorded if line.strip()) + + +def _what_the_rows_say_drew_them(path): + """What every row of a log agrees drew it, or ``None`` if it holds none. ``None`` means the log holds no rows, and nothing else does. Rows that carry no root are refused instead: they cannot be shown to be one study, @@ -117,14 +185,14 @@ def _root_a_log_was_written_with(path): f"cannot continue {path}: a row cannot be read, so what " f"produced it cannot be established." ) from error - if _SIMULATION_ROOT_KEY not in row: + root = row.get(_SIMULATION_ROOT_KEY) if isinstance(row, dict) else None + if root is None: raise ValueError( f"cannot continue {path}: a row does not say which root " f"drew it, which is how a study written before this " f"release looks. Start a new one rather than continuing " f"one whose rows cannot be checked." ) - root = row[_SIMULATION_ROOT_KEY] if first is _NOTHING_READ_YET: first = root elif root != first: @@ -732,16 +800,27 @@ def __continue_the_root_the_rows_carry(self, random_seed): also means a fresh object can continue a study, which is the ordinary way of resuming one. """ - recorded = _root_a_log_was_written_with(self.input_file) + recorded = _what_the_rows_say_drew_them(self.input_file) + stamped = _what_the_rows_say_drew_them(self.output_file) + named = None if recorded is None else _root_digest(recorded) + if named != stamped: + raise ValueError( + f"cannot append to {self.input_file}: it and {self.output_file} " + f"were not drawn from the same root, so they are two studies " + f"rather than the two halves of one." + ) if recorded is None: return - if random_seed is None: - self.__root_state = ( - recorded["entropy"], - tuple(recorded["spawn_key"]), - recorded["pool_size"], - recorded["n_children_spawned"], + held = _rows_a_log_holds(self.input_file) + if held != _rows_a_log_holds(self.output_file): + raise ValueError( + f"cannot append to {self.input_file}: it and {self.output_file} " + f"hold different numbers of rows, so where to carry on from " + f"cannot be established." ) + state = _root_state_a_row_records(recorded, self.input_file) + if random_seed is None: + self.__root_state = state return if _root_written_into_a_row(self.__root_state) != recorded: raise ValueError( @@ -1020,8 +1099,11 @@ def __evaluate_flight_outputs(self, flight, sim_idx): ) from e outputs_dict = outputs_dict | additional_exports - # After the collectors, so one cannot take the row's own number. + # After the collectors: these two say which run the row belongs to. outputs_dict["index"] = sim_idx + outputs_dict[_SIMULATION_ROOT_KEY] = _root_digest( + _root_written_into_a_row(self.__root_state) + ) return ( json.dumps(outputs_dict, cls=RocketPyEncoder, **self._export_config) + "\n" diff --git a/tests/unit/simulation/test_append_lineage.py b/tests/unit/simulation/test_append_lineage.py index aef91add3..ec45194ab 100644 --- a/tests/unit/simulation/test_append_lineage.py +++ b/tests/unit/simulation/test_append_lineage.py @@ -2,7 +2,11 @@ import pytest -from rocketpy.simulation.monte_carlo import MonteCarlo, _SIMULATION_ROOT_KEY +from rocketpy.simulation.monte_carlo import ( + MonteCarlo, + _root_digest, + _SIMULATION_ROOT_KEY, +) def _study(tmp_path, stem, models): @@ -112,6 +116,76 @@ def test_a_log_whose_rows_carry_no_root_is_refused(models, tmp_path): analysis.simulate(4, append=True) +def _rewrite(path, replacement): + """Put ``replacement`` under the root key of every row of one log.""" + with open(path, "r", encoding="utf-8") as written: + rows = [json.loads(line) for line in written if line.strip()] + for row in rows: + row[_SIMULATION_ROOT_KEY] = replacement(row[_SIMULATION_ROOT_KEY]) + with open(path, "w", encoding="utf-8") as rewritten: + for row in rows: + rewritten.write(json.dumps(row) + "\n") + + +def _rewrite_roots(analysis, damage): + """Damage the recorded root, leaving the two logs still naming one study. + + The output rows hold the root's digest, so they are restamped rather than + damaged: otherwise the two logs disagree and that is refused first. + """ + with open(analysis.input_file, "r", encoding="utf-8") as written: + first = json.loads(next(line for line in written if line.strip())) + damaged = damage(first[_SIMULATION_ROOT_KEY]) + _rewrite(analysis.input_file, lambda _root: damaged) + if isinstance(damaged, dict): + _rewrite(analysis.output_file, lambda _digest: _root_digest(damaged)) + + +def test_a_log_whose_root_is_null_is_refused(models, tmp_path): + """A null root is no more of a lineage than no root at all.""" + # It read as an empty log, so an append started a second root behind the + # damaged rows, which is the case this check exists to prevent. + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + + _rewrite_roots(analysis, lambda root: None) + + with pytest.raises(ValueError, match="does not say which root"): + analysis.simulate(4, append=True) + + +@pytest.mark.parametrize( + "damage", + [ + lambda root: {**root, "entropy": None}, + lambda root: {**root, "pool_size": 0}, + lambda root: {**root, "pool_size": 2}, + lambda root: {**root, "n_children_spawned": -1}, + lambda root: {**root, "spawn_key": [True]}, + lambda root: {key: value for key, value in root.items() if key != "entropy"}, + lambda root: {**root, "unexpected": 1}, + ], + ids=[ + "null entropy", + "no pool", + "pool numpy refuses", + "negative base", + "bool key", + "short", + "long", + ], +) +def test_a_root_that_no_stream_can_be_rebuilt_from_is_refused(models, tmp_path, damage): + """Rows agreeing on a root does not make it a root that can be resumed.""" + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + + _rewrite_roots(analysis, damage) + + with pytest.raises(ValueError, match="rebuilt from"): + analysis.simulate(4, append=True) + + def test_an_empty_log_is_not_a_log_that_cannot_be_checked(models, tmp_path): """The control. Nothing recorded yet is a fresh start, not a refusal.""" analysis = _study(tmp_path, "study", models) @@ -168,3 +242,50 @@ def test_a_row_that_cannot_be_read_is_refused(models, tmp_path): with pytest.raises(ValueError, match="cannot be read"): analysis.simulate(4, append=True) + + +def test_an_output_log_from_another_study_is_refused(models, tmp_path): + """Matching indices do not make two files the two halves of one run.""" + # Nothing in the indices tells them apart: both studies number their rows + # from zero, so the root is what says they belong together. + one = _study(tmp_path, "one", models) + one.simulate(2, append=False, random_seed=42) + other = _study(tmp_path, "other", models) + other.simulate(2, append=False, random_seed=7) + with open(other.output_file, "r", encoding="utf-8") as theirs: + stolen = theirs.read() + with open(one.output_file, "w", encoding="utf-8") as ours: + ours.write(stolen) + + with pytest.raises(ValueError, match="same root"): + one.simulate(4, append=True) + + +def test_a_checkpoint_whose_halves_disagree_is_refused(models, tmp_path): + """Where to carry on from is not established by one log alone.""" + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + with open(analysis.output_file, "r", encoding="utf-8") as written: + rows = [line for line in written if line.strip()] + with open(analysis.output_file, "w", encoding="utf-8") as trimmed: + trimmed.write(rows[0]) + + with pytest.raises(ValueError, match="different numbers of rows"): + analysis.simulate(4, append=True) + + +def test_a_collector_cannot_take_over_the_root(models, tmp_path): + """The root binds the two logs, so nothing outside the run writes it.""" + analysis = _study(tmp_path, "study", models) + analysis.data_collector = {_SIMULATION_ROOT_KEY: lambda _flight: "forged"} + + analysis.simulate(2, append=False, random_seed=42) + + with open(analysis.output_file, "r", encoding="utf-8") as written: + roots = { + json.dumps(json.loads(line)[_SIMULATION_ROOT_KEY], sort_keys=True) + for line in written + if line.strip() + } + assert roots != {'"forged"'} + assert len(roots) == 1 From d7e843f2b529085aba07b13736cbccd7228f1e3d Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Fri, 4 Sep 2026 00:06:44 +0800 Subject: [PATCH 15/15] BUG: validate the checkpoint an append carries on from Matching roots and row counts did not establish that two logs hold the same records. One paired scan reads the root and the indices together, requires the two logs to number the same simulations, and requires those to be the run's first n. They are not required to be sorted: a parallel run finishes out of order. Where to carry on from comes from that scan rather than the line count taken when the object was built. That one counts blank lines, and goes stale if the files change after construction. run_root joins index as a name a data collector cannot use. Overwriting it after the callbacks protects the log, but runs a callback and throws the result away. Rebased onto the atomic index claim, which this branch did not carry. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/simulation/monte_carlo.py | 63 ++++++++---- tests/unit/simulation/test_append_lineage.py | 102 ++++++++++++++++++- 2 files changed, 144 insertions(+), 21 deletions(-) diff --git a/rocketpy/simulation/monte_carlo.py b/rocketpy/simulation/monte_carlo.py index 6feba2e79..6694c93eb 100644 --- a/rocketpy/simulation/monte_carlo.py +++ b/rocketpy/simulation/monte_carlo.py @@ -104,8 +104,9 @@ def _root_written_into_a_row(root_state): def _root_digest(root): """A short stable name for a root, for rows that only have to match one. - Carried by the output rows instead of the root itself, which would cost a - hundred bytes a row on a study the input log already records it for. + 64 bits of SHA-256, carried by the output rows instead of the root itself, + which would cost a hundred bytes a row on a study the input log already + records it for. Wide enough to tell studies apart, not a signature. """ canonical = json.dumps(root, sort_keys=True, separators=(",", ":")) return hashlib.sha256(canonical.encode("utf-8")).hexdigest()[:16] @@ -159,14 +160,10 @@ def _root_state_a_row_records(root, path): ) -def _rows_a_log_holds(path): - """How many simulations a log records, blank lines aside.""" - with open(path, "r", encoding="utf-8") as recorded: - return sum(1 for line in recorded if line.strip()) - - def _what_the_rows_say_drew_them(path): - """What every row of a log agrees drew it, or ``None`` if it holds none. + """What every row agrees drew it, and the simulations it numbers. + + ``(None, [])`` means the log holds no rows, and nothing else does. ``None`` means the log holds no rows, and nothing else does. Rows that carry no root are refused instead: they cannot be shown to be one study, @@ -174,6 +171,7 @@ def _what_the_rows_say_drew_them(path): A log whose rows disagree is refused for the same reason. """ first = _NOTHING_READ_YET + numbered = [] with open(path, "r", encoding="utf-8") as recorded: for line in recorded: if not line.strip(): @@ -193,6 +191,14 @@ def _what_the_rows_say_drew_them(path): f"release looks. Start a new one rather than continuing " f"one whose rows cannot be checked." ) + index = row.get("index") + if not _a_whole_number(index): + raise ValueError( + f"cannot continue {path}: a row does not number the " + f"simulation it holds, so where to carry on from cannot " + f"be established." + ) + numbered.append(index) if first is _NOTHING_READ_YET: first = root elif root != first: @@ -200,7 +206,7 @@ def _what_the_rows_say_drew_them(path): f"cannot continue {path}: its rows were not all drawn " f"from one root, so it holds more than one study." ) - return None if first is _NOTHING_READ_YET else first + return (None if first is _NOTHING_READ_YET else first), numbered def _root_state_of(root): @@ -521,7 +527,7 @@ def simulate( """ self._export_config = kwargs self.number_of_simulations = number_of_simulations - self._initial_sim_idx = self.num_of_loaded_sims if append else 0 + self._initial_sim_idx = 0 # Validated here, before __setup_files truncates anything, so an # unusable seed cannot cost a previous run its results. Kept as four # picklable values rather than as the object, since a worker rebuilds @@ -535,7 +541,9 @@ def simulate( # After that one, which says plainly that a .csv cannot be a working # log. Reaching this first would report it as a row that cannot be read. if append: - self.__continue_the_root_the_rows_carry(random_seed) + # From the checkpoint just validated, not the line count taken when + # this object was built: that one counts blank lines and goes stale. + self._initial_sim_idx = self.__continue_the_root_the_rows_carry(random_seed) print("Starting Monte Carlo analysis") @@ -800,8 +808,8 @@ def __continue_the_root_the_rows_carry(self, random_seed): also means a fresh object can continue a study, which is the ordinary way of resuming one. """ - recorded = _what_the_rows_say_drew_them(self.input_file) - stamped = _what_the_rows_say_drew_them(self.output_file) + recorded, held = _what_the_rows_say_drew_them(self.input_file) + stamped, written = _what_the_rows_say_drew_them(self.output_file) named = None if recorded is None else _root_digest(recorded) if named != stamped: raise ValueError( @@ -810,18 +818,25 @@ def __continue_the_root_the_rows_carry(self, random_seed): f"rather than the two halves of one." ) if recorded is None: - return - held = _rows_a_log_holds(self.input_file) - if held != _rows_a_log_holds(self.output_file): + return 0 + if held != written: raise ValueError( f"cannot append to {self.input_file}: it and {self.output_file} " - f"hold different numbers of rows, so where to carry on from " - f"cannot be established." + f"do not record the same simulations, so where to carry on " + f"from cannot be established." + ) + # Rows arrive in completion order, so these are not sorted. What has + # to hold is that between them they are the run's first len(held). + if sorted(held) != list(range(len(held))): + raise ValueError( + f"cannot append to {self.input_file}: the simulations it " + f"records are not the run's first {len(held)}, so where to " + f"carry on from cannot be established." ) state = _root_state_a_row_records(recorded, self.input_file) if random_seed is None: self.__root_state = state - return + return len(held) if _root_written_into_a_row(self.__root_state) != recorded: raise ValueError( f"cannot append to {self.input_file}: its rows were drawn from " @@ -829,6 +844,7 @@ def __continue_the_root_the_rows_carry(self, random_seed): f"two studies in one file. Pass the seed the run started with, " f"or leave random_seed out to carry on from the rows." ) + return len(held) def __seed_this_simulation(self, sim_idx): """Reseed the three models from this index's own child of the root. @@ -1252,6 +1268,13 @@ def _check_data_collector(self, data_collector): "Invalid 'data_collector' key! " f"Variable names overwrites 'export_list' key '{key}'." ) + if key == _SIMULATION_ROOT_KEY: + raise ValueError( + f"Invalid 'data_collector' key '{key}'! It is the root " + f"the row was drawn with, which is written after the " + f"collectors run, so a callback under that name would " + f"be run and then discarded." + ) if not callable(callback): raise ValueError( f"Invalid value in 'data_collector' for key '{key}'! " diff --git a/tests/unit/simulation/test_append_lineage.py b/tests/unit/simulation/test_append_lineage.py index ec45194ab..35f80844b 100644 --- a/tests/unit/simulation/test_append_lineage.py +++ b/tests/unit/simulation/test_append_lineage.py @@ -270,12 +270,13 @@ def test_a_checkpoint_whose_halves_disagree_is_refused(models, tmp_path): with open(analysis.output_file, "w", encoding="utf-8") as trimmed: trimmed.write(rows[0]) - with pytest.raises(ValueError, match="different numbers of rows"): + with pytest.raises(ValueError, match="do not record the same simulations"): analysis.simulate(4, append=True) def test_a_collector_cannot_take_over_the_root(models, tmp_path): """The root binds the two logs, so nothing outside the run writes it.""" + # Set past the constructor, so this pins the row and not the validation. analysis = _study(tmp_path, "study", models) analysis.data_collector = {_SIMULATION_ROOT_KEY: lambda _flight: "forged"} @@ -289,3 +290,102 @@ def test_a_collector_cannot_take_over_the_root(models, tmp_path): } assert roots != {'"forged"'} assert len(roots) == 1 + + +def test_a_collector_named_after_the_root_is_refused(models, tmp_path): + """Overwriting it afterwards protects the log but discards the callback.""" + with pytest.raises(ValueError, match="run after the collectors|discarded"): + MonteCarlo( + filename=str(tmp_path / "study"), + environment=models[0], + rocket=models[1], + flight=models[2], + data_collector={_SIMULATION_ROOT_KEY: lambda _flight: 1}, + ) + + +def _rows_of(path): + with open(path, "r", encoding="utf-8") as written: + return [line for line in written if line.strip()] + + +def _rewrite_indices(path, numbers): + """Renumber a log's rows, leaving everything else in them alone.""" + rows = [json.loads(line) for line in _rows_of(path)] + for row, number in zip(rows, numbers): + row["index"] = number + with open(path, "w", encoding="utf-8") as rewritten: + for row in rows: + rewritten.write(json.dumps(row) + "\n") + + +def test_a_blank_line_in_the_output_log_does_not_move_the_continuation( + models, tmp_path +): + """Where to carry on from comes from the records, not the line count.""" + # A fresh object counts physical output lines when it opens the file, so + # one blank line made an append start at 3 and leave index 2 missing. + written = _study(tmp_path, "study", models) + written.simulate(2, append=False, random_seed=42) + with open(written.output_file, "a", encoding="utf-8") as padded: + padded.write("\n") + + resumed = _study(tmp_path, "study", models) + assert resumed.num_of_loaded_sims == 3 # the count this must not trust + resumed.simulate(4, append=True) + + assert sorted(_drawn(resumed)) == [0, 1, 2, 3] + + +def test_halves_that_number_the_same_count_differently_are_refused(models, tmp_path): + """Equal row counts do not make two logs the two halves of one run.""" + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + _rewrite_indices(analysis.output_file, [0, 2]) + + with pytest.raises(ValueError, match="do not record the same simulations"): + analysis.simulate(4, append=True) + + +@pytest.mark.parametrize( + "numbers", [[0, 0], [0, 2], [1, 2]], ids=["duplicate", "hole", "no zero"] +) +def test_a_checkpoint_that_is_not_the_runs_first_simulations_is_refused( + models, tmp_path, numbers +): + """An append continues a run, so what it continues has to be its start.""" + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + for path in (analysis.input_file, analysis.output_file): + _rewrite_indices(path, numbers) + + with pytest.raises(ValueError, match="not the run's first"): + analysis.simulate(4, append=True) + + +def test_the_order_a_parallel_run_finished_in_is_carried_on_from(models, tmp_path): + """Rows arrive in completion order, which is not sorted and not wrong.""" + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + for path in (analysis.input_file, analysis.output_file): + _rewrite_indices(path, [1, 0]) + + analysis.simulate(4, append=True) + + assert sorted(_drawn(analysis)) == [0, 1, 2, 3] + + +@pytest.mark.parametrize( + "number", [True, 1.0, "1", None, -1], ids=["bool", "float", "str", "null", "neg"] +) +def test_a_row_that_does_not_number_its_simulation_is_refused(models, tmp_path, number): + """Only a non-negative int says which simulation a row holds.""" + # True and 1.0 both equal 1, so either would pass for a record that is not + # there, and the count they contribute to decides where an append starts. + analysis = _study(tmp_path, "study", models) + analysis.simulate(2, append=False, random_seed=42) + for path in (analysis.input_file, analysis.output_file): + _rewrite_indices(path, [0, number]) + + with pytest.raises(ValueError, match="does not number"): + analysis.simulate(4, append=True)