Skip to content

Commit da2ba5c

Browse files
committed
BUG: validate the run seed before truncating output; tidy the seed helper
simulate() set up (and, for append=False, truncated with w+) the input/output/error files before the seed was validated, so passing a rejected seed such as a Generator destroyed a previous run's results on the way to raising a TypeError. The seed is now captured and validated before __setup_files runs. Moved _seed_sequence_to_int to rocketpy.tools so the stochastic models can share it, and corrected its docstring: a 128-bit int is accepted by default_rng and random.Random, but the legacy RandomState caps a single-int seed at 2**32-1, so the earlier 'accepted by RandomState' claim was wrong. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
1 parent 4740777 commit da2ba5c

3 files changed

Lines changed: 71 additions & 36 deletions

File tree

rocketpy/simulation/monte_carlo.py

Lines changed: 18 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
from rocketpy.prints.monte_carlo_prints import _MonteCarloPrints
3131
from rocketpy.simulation.flight import Flight
3232
from rocketpy.tools import (
33+
_seed_sequence_to_int,
3334
generate_monte_carlo_ellipses,
3435
generate_monte_carlo_ellipses_coordinates,
3536
import_optional_dependency,
@@ -240,18 +241,22 @@ def simulate(
240241
self._export_config = kwargs
241242
self.number_of_simulations = number_of_simulations
242243
self._initial_sim_idx = self.num_of_loaded_sims if append else 0
243-
# Small, picklable root seed state captured once per run; every
244-
# simulation index derives its child seed from it (see __child_seed).
245-
self.__root_state = None
244+
245+
# Capture the small, picklable root seed state once per run (every
246+
# simulation index derives its child seed from it, see __child_seed).
247+
# This validates random_seed *before* __setup_files truncates any
248+
# existing output, so an invalid seed cannot destroy prior results on
249+
# the way to raising.
250+
self.__capture_root_state(random_seed)
246251

247252
print("Starting Monte Carlo analysis")
248253

249254
self.__setup_files(append)
250255

251256
if parallel:
252-
self.__run_in_parallel(n_workers, random_seed)
257+
self.__run_in_parallel(n_workers)
253258
else:
254-
self.__run_in_serial(random_seed)
259+
self.__run_in_serial()
255260

256261
self.__terminate_simulation()
257262

@@ -359,14 +364,12 @@ def __seed_simulation(self, child_seed):
359364
self.rocket._set_stochastic(_seed_sequence_to_int(rocket_seed))
360365
self.flight._set_stochastic(_seed_sequence_to_int(flight_seed))
361366

362-
def __run_in_serial(self, random_seed=None): # pylint: disable=too-many-statements
367+
def __run_in_serial(self): # pylint: disable=too-many-statements
363368
"""
364369
Runs the monte carlo simulation in serial mode.
365370
366-
Parameters
367-
----------
368-
random_seed : int or SeedSequence, optional
369-
Root seed for the run. See ``simulate``.
371+
The root seed state is captured by ``simulate`` before this runs, so each
372+
simulation index derives its child seed from ``self.__root_state``.
370373
371374
Returns
372375
-------
@@ -377,7 +380,6 @@ def __run_in_serial(self, random_seed=None): # pylint: disable=too-many-stateme
377380
n_simulations=self.number_of_simulations,
378381
start_time=time(),
379382
)
380-
self.__capture_root_state(random_seed)
381383
try:
382384
while sim_monitor.keep_simulating():
383385
sim_idx = sim_monitor.increment() - 1
@@ -408,17 +410,19 @@ def __run_in_serial(self, random_seed=None): # pylint: disable=too-many-stateme
408410
f.write(inputs_json)
409411
raise error
410412

411-
def __run_in_parallel(self, n_workers=None, random_seed=None):
413+
def __run_in_parallel(self, n_workers=None):
412414
"""
413415
Runs the monte carlo simulation in parallel.
414416
417+
The root seed state is captured by ``simulate`` before this runs and
418+
travels with the pickled instance, so every worker derives the same
419+
per-index child seed from ``self.__root_state``.
420+
415421
Parameters
416422
----------
417423
n_workers: int, optional
418424
Number of workers to be used. If None, the number of workers
419425
will be equal to the number of CPUs available. Default is None.
420-
random_seed : int or SeedSequence, optional
421-
Root seed for the run. See ``simulate``.
422426
423427
Returns
424428
-------
@@ -446,8 +450,6 @@ def __run_in_parallel(self, n_workers=None, random_seed=None):
446450
# the sampled inputs do not depend on the number of workers. The
447451
# root state is small and travels with the pickled instance, so no
448452
# per-index seed list is materialized or sent to each process.
449-
self.__capture_root_state(random_seed)
450-
451453
for _ in range(n_workers):
452454
sim_producer = multiprocess.Process(
453455
target=self.__sim_producer,
@@ -1710,26 +1712,6 @@ def export_errors_to_json(self, filename):
17101712
self._write_log_to_json(self.errors_log, filename)
17111713

17121714

1713-
def _seed_sequence_to_int(seed_sequence):
1714-
"""Collapse a ``SeedSequence`` into a 128-bit Python ``int``.
1715-
1716-
A plain ``int`` is the one seed type accepted alike by
1717-
``numpy.random.default_rng``, ``numpy.random.RandomState`` and the stdlib
1718-
``random.Random`` (which rejects a ``SeedSequence`` with a ``TypeError``
1719-
since Python 3.11), so a custom sampler whose ``reset_seed`` documents an
1720-
``int`` keeps working. All four ``uint32`` words are combined to keep the
1721-
full 128-bit pool, so the environment/rocket/flight sub-streams stay
1722-
decorrelated instead of collapsing to a single 32-bit word.
1723-
1724-
The words are combined by value (little-endian word order), not via
1725-
``tobytes()``, so the seed is the same on big- and little-endian machines
1726-
-- a byte-order-dependent seed would break the cross-platform
1727-
reproducibility this whole scheme exists to provide.
1728-
"""
1729-
words = seed_sequence.generate_state(4, dtype=np.uint32)
1730-
return sum(int(word) << (32 * position) for position, word in enumerate(words))
1731-
1732-
17331715
def _claim_next_index(sim_monitor, mutex):
17341716
"""Atomically claim the next 0-based simulation index, or ``None`` if done.
17351717

rocketpy/tools.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,6 +1467,29 @@ def find_obj_from_hash(obj, hash_, depth_limit=None):
14671467
return None
14681468

14691469

1470+
def _seed_sequence_to_int(seed_sequence):
1471+
"""Collapse a ``SeedSequence`` into a 128-bit Python ``int``.
1472+
1473+
A plain ``int`` is what ``numpy.random.default_rng`` and the stdlib
1474+
``random.Random`` both accept (``random.Random`` rejects a ``SeedSequence``
1475+
with a ``TypeError`` since Python 3.11), so a custom sampler whose
1476+
``reset_seed`` documents an ``int`` and builds a modern generator keeps
1477+
working. The legacy ``numpy.random.RandomState`` is the exception: it caps a
1478+
single-integer seed at ``2**32 - 1``, so a sampler still built on it would
1479+
have to reduce the value (``RandomState`` is a frozen legacy API NumPy steers
1480+
new code away from). All four ``uint32`` words are combined to keep the full
1481+
128-bit pool, so sub-streams stay decorrelated instead of collapsing to a
1482+
single 32-bit word.
1483+
1484+
The words are combined by value (little-endian word order), not via
1485+
``tobytes()``, so the seed is the same on big- and little-endian machines --
1486+
a byte-order-dependent seed would break the cross-platform reproducibility
1487+
this exists to provide.
1488+
"""
1489+
words = seed_sequence.generate_state(4, dtype=np.uint32)
1490+
return sum(int(word) << (32 * position) for position, word in enumerate(words))
1491+
1492+
14701493
if __name__ == "__main__": # pragma: no cover
14711494
import doctest
14721495

tests/integration/simulation/test_monte_carlo_determinism.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,36 @@ def _simulate_inputs(
150150
return _read_inputs_by_index(montecarlo.input_file)
151151

152152

153+
def test_invalid_seed_does_not_truncate_existing_output(
154+
monkeypatch,
155+
tmp_path,
156+
stochastic_environment,
157+
stochastic_calisto_numpy_only,
158+
stochastic_flight,
159+
):
160+
"""A rejected seed must fail before any output file is truncated, so passing
161+
an invalid seed cannot destroy the results of a previous run."""
162+
monkeypatch.setattr(mc_module, "Flight", _StubFlight)
163+
montecarlo = MonteCarlo(
164+
filename=str(tmp_path / "keep"),
165+
environment=stochastic_environment,
166+
rocket=stochastic_calisto_numpy_only,
167+
flight=stochastic_flight,
168+
)
169+
with open(montecarlo.input_file, "w", encoding="utf-8") as existing:
170+
existing.write("previous results\n")
171+
172+
# A Generator is not a seed and is rejected; the run must raise before the
173+
# ``w+`` file setup truncates anything.
174+
with pytest.raises(TypeError):
175+
montecarlo.simulate(
176+
number_of_simulations=3, random_seed=np.random.default_rng(0)
177+
)
178+
179+
with open(montecarlo.input_file, encoding="utf-8") as kept:
180+
assert kept.read() == "previous results\n"
181+
182+
153183
def test_serial_inputs_are_reproducible(
154184
monkeypatch,
155185
tmp_path,

0 commit comments

Comments
 (0)