Skip to content

Commit 7b2217a

Browse files
committed
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 d21abde^ in 2.32s and does not finish on develop: the worker's own error path raises UnboundLocalError on inputs_json, so the parent never learns it died and the run hangs. The children of one root share their entropy and differ by spawn_key, so the value is folded through generate_state rather than read off entropy, which would put every worker on one sampler stream. Nothing is consumed, and an int or None seed keeps the stream it had. The fold lives in rocketpy.tools, since the component streams and the per-index seeding both need the same one and three copies would drift on width and word order. _sampler_seed does its own final fold through it as well rather than repeating the four lines. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
1 parent 4263fa9 commit 7b2217a

4 files changed

Lines changed: 140 additions & 4 deletions

File tree

‎rocketpy/stochastic/stochastic_model.py‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from rocketpy.mathutils.function import Function
99
from rocketpy.stochastic.custom_sampler import CustomSampler
1010

11-
from ..tools import get_distribution
11+
from ..tools import _seed_sequence_to_int, get_distribution
1212

1313

1414
def _names_as_spawn_key(input_names):
@@ -41,6 +41,18 @@ def _format_number(value):
4141
return f"array of shape {np.shape(value)}"
4242

4343

44+
def _seed_as_entropy(seed):
45+
"""A seed as something ``SeedSequence`` will take as entropy.
46+
47+
A parallel run is handed a ``SeedSequence``, which it will not take. Any
48+
other seed goes through untouched, so the stream an int reaches stays where
49+
it was.
50+
"""
51+
if not isinstance(seed, np.random.SeedSequence):
52+
return seed
53+
return _seed_sequence_to_int(seed)
54+
55+
4456
def _sampler_seed(seed, input_names):
4557
"""Derive a seed for one sampler, or for one group that shares a generator.
4658
@@ -54,10 +66,10 @@ def _sampler_seed(seed, input_names):
5466
# Sorted here rather than trusting the caller, so a future call site cannot
5567
# give one group two different seeds by listing its members another way.
5668
root = np.random.SeedSequence(
57-
entropy=seed, spawn_key=_names_as_spawn_key(tuple(sorted(input_names)))
69+
entropy=_seed_as_entropy(seed),
70+
spawn_key=_names_as_spawn_key(tuple(sorted(input_names))),
5871
)
59-
words = root.generate_state(4, dtype=np.uint32)
60-
return sum(int(word) << (32 * position) for position, word in enumerate(words))
72+
return _seed_sequence_to_int(root)
6173

6274

6375
# TODO: Stop using assert in production code. Use exceptions instead.

‎rocketpy/tools.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,6 +1377,17 @@ def euler313_to_quaternions(phi, theta, psi):
13771377
return e0, e1, e2, e3
13781378

13791379

1380+
def _seed_sequence_to_int(seed_sequence):
1381+
"""Returns a ``SeedSequence`` as the 128-bit ``int`` it can be rebuilt from.
1382+
1383+
Folded through ``generate_state`` rather than read off ``entropy``, since
1384+
the children of one root differ only by ``spawn_key``, and combined by
1385+
value so it does not depend on byte order.
1386+
"""
1387+
words = seed_sequence.generate_state(4, dtype=np.uint32)
1388+
return sum(int(word) << (32 * position) for position, word in enumerate(words))
1389+
1390+
13801391
def get_matplotlib_supported_file_endings():
13811392
"""Gets the file endings supported by matplotlib.
13821393
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import pytest
2+
3+
from rocketpy.simulation.monte_carlo import MonteCarlo
4+
5+
6+
@pytest.mark.parametrize("parallel", [False, True])
7+
def test_a_monte_carlo_run_finishes(
8+
stochastic_environment, stochastic_calisto, stochastic_flight, tmp_path, parallel
9+
):
10+
# The parallel path hands each worker a SeedSequence rather than an int, and
11+
# nothing else in the suite exercises that. A worker that dies on it is not
12+
# reported, so this reads as a hang rather than as a failure.
13+
#
14+
# Built here rather than taken from the monte_carlo_calisto fixture, whose
15+
# own filename is fixed, since `filename` is a plain attribute and the three
16+
# working paths are settled when the object is constructed.
17+
analysis = MonteCarlo(
18+
filename=str(tmp_path / "study"),
19+
environment=stochastic_environment,
20+
rocket=stochastic_calisto,
21+
flight=stochastic_flight,
22+
)
23+
24+
analysis.simulate(
25+
number_of_simulations=2,
26+
append=False,
27+
parallel=parallel,
28+
n_workers=2 if parallel else None,
29+
)
30+
31+
assert analysis.num_of_loaded_sims == 2
32+
assert str(tmp_path) in str(analysis.output_file)
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import numpy as np
2+
import pytest
3+
4+
from rocketpy.stochastic.stochastic_model import (
5+
_names_as_spawn_key,
6+
_sampler_seed,
7+
)
8+
from rocketpy.tools import _seed_sequence_to_int
9+
10+
11+
def _a_worker_seed(index=0, workers=2):
12+
# What MonteCarlo.__run_in_parallel spawns and hands to each worker, which
13+
# passes it straight to environment/rocket/flight._set_stochastic.
14+
return np.random.SeedSequence().spawn(workers)[index]
15+
16+
17+
def test_the_seed_type_a_worker_is_handed_is_accepted(stochastic_calisto):
18+
stochastic_calisto._set_stochastic(_a_worker_seed())
19+
20+
stochastic_calisto.create_object()
21+
22+
23+
def test_a_parachute_derives_its_noise_seed_from_a_worker_seed(
24+
stochastic_main_parachute,
25+
):
26+
stochastic_main_parachute._set_stochastic(_a_worker_seed())
27+
28+
assert stochastic_main_parachute.create_object().noise[2] is not None
29+
30+
31+
def test_two_workers_do_not_share_a_sampler_stream():
32+
first, second = np.random.SeedSequence(7).spawn(2)
33+
# They come off one root, so they carry the same entropy and differ only in
34+
# spawn_key. Reading the entropy alone would put both on one stream.
35+
assert first.entropy == second.entropy
36+
37+
assert _sampler_seed(first, ("__list_choice__",)) != _sampler_seed(
38+
second, ("__list_choice__",)
39+
)
40+
41+
42+
def test_a_caller_seed_sequence_is_not_consumed():
43+
root = np.random.SeedSequence(42)
44+
45+
_sampler_seed(root, ("__list_choice__",))
46+
47+
assert root.n_children_spawned == 0
48+
assert root.spawn(1)[0].spawn_key == (0,)
49+
50+
51+
def test_the_same_seed_sequence_twice_gives_the_same_sampler_seed():
52+
root = np.random.SeedSequence(42)
53+
54+
first = _sampler_seed(root, ("pressure_noise", "main"))
55+
second = _sampler_seed(root, ("pressure_noise", "main"))
56+
57+
assert first == second
58+
59+
60+
@pytest.mark.parametrize("seed", [42, 7, [1, 2, 3]])
61+
@pytest.mark.parametrize("names", [("__list_choice__",), ("pressure_noise", "main")])
62+
def test_a_seed_that_is_not_a_sequence_reaches_numpy_untouched(seed, names):
63+
# The control. Every fixed-seed baseline in the suite was recorded through
64+
# this path, so anything but a SeedSequence has to arrive as it always did.
65+
# Compared with the expression rather than with a recorded number, which
66+
# would go red on a NumPy release instead of on a change of ours.
67+
unchanged = np.random.SeedSequence(
68+
entropy=seed, spawn_key=_names_as_spawn_key(tuple(sorted(names)))
69+
)
70+
71+
assert _sampler_seed(seed, names) == _seed_sequence_to_int(unchanged)
72+
73+
74+
def test_no_seed_still_means_no_seed():
75+
# None is left out above on purpose: it asks NumPy for fresh entropy, so
76+
# two calls must not agree, and comparing one against another would be
77+
# asserting the opposite of what an unseeded run promises.
78+
first = _sampler_seed(None, ("__list_choice__",))
79+
second = _sampler_seed(None, ("__list_choice__",))
80+
81+
assert first != second

0 commit comments

Comments
 (0)