From 7b2217a5f528843f6fae80ade8f456c16d4dc1d7 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] 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