From 271c53e7747943dd6777d593e0d3db7705a572e9 Mon Sep 17 00:00:00 2001 From: Gui-FernandesBR Date: Wed, 4 Nov 2026 13:07:17 -0300 Subject: [PATCH 1/2] BUG: fix Monte Carlo release blockers Addresses the Monte Carlo fixes in https://github.com/RocketPy-Team/RocketPy/issues/1209. Simplifies study seeds and checkpoint recovery, preserves stochastic configurations, and adds regression coverage and migration notes. --- CHANGELOG.md | 11 +- docs/TODO.md | 5 + docs/user/stochastic.rst | 37 +- rocketpy/simulation/monte_carlo.py | 430 ++++++----------- rocketpy/stochastic/custom_sampler.py | 4 +- rocketpy/stochastic/stochastic_flight.py | 18 +- rocketpy/stochastic/stochastic_model.py | 5 +- rocketpy/stochastic/stochastic_solid_motor.py | 1 + .../simulation/test_monte_carlo_release.py | 166 +++++++ tests/unit/simulation/test_append_lineage.py | 437 ++++-------------- tests/unit/simulation/test_monte_carlo.py | 36 +- .../simulation/test_monte_carlo_seeding.py | 99 ++-- .../test_monte_carlo_simulation_index.py | 22 +- .../test_monte_carlo_worker_reporting.py | 6 +- tests/unit/stochastic/test_custom_sampler.py | 14 +- .../stochastic/test_release_configuration.py | 64 +++ 16 files changed, 602 insertions(+), 753 deletions(-) create mode 100644 docs/TODO.md create mode 100644 tests/integration/simulation/test_monte_carlo_release.py create mode 100644 tests/unit/stochastic/test_release_configuration.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5631b7494..2cefb0991 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,8 +30,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/). +### Breaking changes + +- New serial Monte Carlo studies use indices `0..n-1` instead of `1..n`, matching parallel studies. Legacy logs retain their existing numbering when extended. [#1187](https://github.com/RocketPy-Team/RocketPy/pull/1187) +- Monte Carlo reseeds stochastic objects for every simulation in both execution modes. Pass a non-negative integer `random_seed` to reproduce a study; individual object seeds are overridden, and fixed-seed results from v1.13 change. New input rows record one integer `seed`, while output rows contain only the index and requested results. Metadata is excluded from statistical results and plots. [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) + ### Added +- ENH: Accept `random_seed` in `MonteCarlo.simulate_convergence`, including across batches. [#1198](https://github.com/RocketPy-Team/RocketPy/pull/1198) [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) - ENH: Reproducible Monte Carlo seeding, and an append that continues the same study [#1187](https://github.com/RocketPy-Team/RocketPy/pull/1187) [#1053](https://github.com/RocketPy-Team/RocketPy/issues/1053) [#1075](https://github.com/RocketPy-Team/RocketPy/issues/1075) - ENH: 3-DOF single rail button flight phase (tip-off analysis), enabled with the opt-in `Flight(use_udot_rail2=True)`. Between the upper rail button leaving the rail and the lower one following it, the rocket pivots about the lower button under a solved constraint wrench instead of jumping straight to free 6-DOF flight. The window is reported as `between_rails_time`, `between_rails_velocity` and `tip_off_duration`, printed by `Flight.info()` and shaded in the attitude plots. Requires `simulation_mode="6 DOF"` and `equations_of_motion="standard"`. [#920](https://github.com/RocketPy-Team/RocketPy/pull/920) - 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) @@ -50,7 +56,7 @@ Attention: The newest changes should be on top --> ### Changed -- ENH: Number the simulations of a serial Monte Carlo run from zero, as the parallel path already did — the two used to name the same simulation `1, 2, 3` and `0, 1, 2`. The `index` field written into the inputs and outputs rows shifts by one for serial runs, so code reading indices off those files has to expect a run of `n` simulations to be numbered `0` to `n - 1`. An append onto a study written before this release is refused rather than continued, since its rows do not record which root drew them. [#1187](https://github.com/RocketPy-Team/RocketPy/pull/1187) +- ENH: Number new serial and parallel Monte Carlo runs from zero. Legacy checkpoints retain their numbering when extended, with a warning that their original random draws cannot be reproduced. [#1187](https://github.com/RocketPy-Team/RocketPy/pull/1187) [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) - ENH: Compute the rocket static margin lazily [#1135](https://github.com/RocketPy-Team/RocketPy/pull/1135) [#780](https://github.com/RocketPy-Team/RocketPy/issues/780) - DOC: Tighten the comments that came with the sampler seed groups [#1154](https://github.com/RocketPy-Team/RocketPy/pull/1154) - CI: make the Gemini PR reviewer actually review [#1140](https://github.com/RocketPy-Team/RocketPy/pull/1140) @@ -61,6 +67,7 @@ Attention: The newest changes should be on top --> ### Fixed +- BUG: Keep checkpoint metadata out of Monte Carlo plots and statistics, recover missing simulation indices without repeating completed flights, and warn when appending v1.13 logs. Preserve explicit initial flight states and solid-motor grain bonding in stochastic models. [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) - BUG: A `Flight` continued from another `Flight` object now records `t_initial`, so a rocket carrying sensors or controllers no longer raises `AttributeError` on that path. [#920](https://github.com/RocketPy-Team/RocketPy/pull/920) - BUG: Correct the gravity sign an `Accelerometer` applies when `consider_gravity=True`. The gravitational field was added to the inertial acceleration instead of subtracted from it, so the sensor reported the negative of the proper acceleration along the vertical: one at rest read -g rather than +g. Recorded accelerometer data taken with `consider_gravity=True` changes sign in that term. [#1175](https://github.com/RocketPy-Team/RocketPy/pull/1175) - BUG: Report a Monte Carlo worker that fails instead of hanging or passing for a finished run [#1182](https://github.com/RocketPy-Team/RocketPy/pull/1182) @@ -84,7 +91,7 @@ Attention: The newest changes should be on top --> - BUG: Restore the UTC fallback without timezonefinder [#1143](https://github.com/RocketPy-Team/RocketPy/pull/1143) - BUG: Pick between the candidate values of a list input with the stochastic model's own seeded generator. `random.choice` was used, which draws from the interpreter-wide stream that `_set_stochastic` does not reseed, so a fixed seed did not reproduce the values chosen from a list, and Monte Carlo workers forked from one process walked a single shared stream instead of sampling independently. Fixed-seed baselines that vary a list input change. [#953](https://github.com/RocketPy-Team/RocketPy/issues/953) - BUG: Report the atmospheric model time period and ensemble member count for lower-case model types. `set_atmospheric_model` documents `type` as case-insensitive, but `Environment.info()` and `all_info()` compared against capitalised literals, so `type="ensemble"` printed no time period and no member count, and skipped the ensemble comparison plot. [#520](https://github.com/RocketPy-Team/RocketPy/issues/520) -- BUG: Give each `CustomSampler` input its own deterministic stream, and seed samplers sharing one generator once as a group. Existing fixed-seed `CustomSampler` baselines change, and samplers built on the legacy `RandomState` must move to `default_rng` because seeds now carry the full 128 bits. [#1102](https://github.com/RocketPy-Team/RocketPy/pull/1102) +- BUG: Give each `CustomSampler` input its own deterministic stream, and seed samplers sharing one generator once as a group. Reset callbacks receive a 32-bit integer compatible with both `RandomState` and `default_rng`; existing fixed-seed baselines change. [#1102](https://github.com/RocketPy-Team/RocketPy/pull/1102) [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) - BUG: Accept the callable parachute triggers `StochasticParachute` documents, and reject the ones it cannot mean. An invalid string, an empty list or a boolean now fails during validation instead of reaching `Parachute` or becoming a one-metre height trigger. [#1103](https://github.com/RocketPy-Team/RocketPy/pull/1103) - BUG: Accept a deterministic aerodynamic surface in `StochasticRocket.add_nose`, `add_trapezoidal_fins`, `add_elliptical_fins` and `add_tail`. Each wrapped the surface with a `component=` keyword none of the stochastic classes accept, so passing anything other than an already-stochastic surface raised a `TypeError`. [#953](https://github.com/RocketPy-Team/RocketPy/issues/953) - BUG: rocket with a late-starting thrust curve never leaves the rail [#1085](https://github.com/RocketPy-Team/RocketPy/pull/1085) diff --git a/docs/TODO.md b/docs/TODO.md new file mode 100644 index 000000000..c4ab6fe9e --- /dev/null +++ b/docs/TODO.md @@ -0,0 +1,5 @@ +# Monte Carlo follow-ups + +The v1.14 fixes tracked in [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) +retain the current worker orchestration. A pool rewrite and validation once per +study remain deferred to v1.15, as agreed in the release review. diff --git a/docs/user/stochastic.rst b/docs/user/stochastic.rst index ed8f8ab10..edb054d35 100644 --- a/docs/user/stochastic.rst +++ b/docs/user/stochastic.rst @@ -316,11 +316,9 @@ on equal values, and a specification with no spread always will. The reset is what builds the tree, not ``add_parachute`` or ``add_nose``. A rocket resets itself once while being constructed, when it has no components yet, so anything attached afterwards keeps the generator it was built with until -something resets it again. A serial ``MonteCarlo`` run does not reset it at -all. A parallel one tries to, once per worker, but hands the model a -``SeedSequence`` where an integer is wanted, so that path does not get as far -as building the tree either. Resetting per simulation, from an integer seed the -caller chooses, is what the Monte Carlo seeding work adds. +something resets it again. ``MonteCarlo`` resets the tree before every +simulation in both serial and parallel runs, using the study seed and the +simulation index to derive independent streams. Each collection is spawned separately, so adding an aerodynamic surface does not move what the parachutes draw. Within a collection the stream follows insertion @@ -348,10 +346,33 @@ draws under a fixed seed. The rocket's own inputs, such as ``mass`` and split over any number of workers, and whether it was reached first or last. A run of ``n`` simulations numbers them 0 to ``n - 1`` either way, which is what makes one index name one simulation. - Every input row records the root it came from, so appending carries that + Every input row records one integer ``seed``, 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. + refused rather than mixed in. Without a seed, a new run generates and prints + one. Pass that integer as ``random_seed`` to reproduce the study. Seeds on + individual stochastic models are overridden; a new study without + ``random_seed`` warns when these models have already been seeded. + + ``simulate_convergence(random_seed=...)`` uses the same study seed for every + batch. Changing the batch size does not change the inputs of a given index. + +Resuming a study +---------------- + +Use ``simulate(number_of_simulations=100, append=True)`` to reach a total of +100 simulations. A completed index is kept; missing indices below the target +are simulated, including gaps left by interrupted parallel workers. Keep the +input and output logs together: they are checked for matching indices and +duplicate records before any new rows are written. The study's models must +still describe the same rocket, flight and environment. + +Logs from v1.13 have no study seed. Appending preserves their rows and emits a +warning: the old random stream cannot be recovered. New rows record the reported +seed. Existing one-based serial numbering is preserved; new studies start at +zero. Checkpoint fields (``index``, ``seed`` and the earlier ``run_root`` field) +are excluded from ``results``, summary statistics and plots. + +.. note:: An index fixes the draw, not the object it is drawn around. Where the note above says a value follows its deterministic object, moving that object diff --git a/rocketpy/simulation/monte_carlo.py b/rocketpy/simulation/monte_carlo.py index 5166920bd..6cae9d030 100644 --- a/rocketpy/simulation/monte_carlo.py +++ b/rocketpy/simulation/monte_carlo.py @@ -14,14 +14,12 @@ """ import csv -import hashlib import json import os import threading import traceback import warnings from contextlib import suppress -from copy import deepcopy from numbers import Real from pathlib import Path from time import monotonic, time @@ -61,132 +59,27 @@ # and leaving at the end of the simulation in hand, not blocked on a dead lock. _REPORTED_FAILURE_GRACE_SECONDS = 60.0 -# Which root drew a row. An append reads it to continue the same stream. -_SIMULATION_ROOT_KEY = "run_root" +_SIMULATION_SEED_KEY = "seed" +_RESULT_METADATA = frozenset(("index", "seed", "run_root")) -# Told apart from a row that carries ``None``, which no run writes. -_NOTHING_READ_YET = object() +def _study_seed(random_seed): + """Validate a study seed before opening or truncating any logs.""" + if random_seed is None: + return int(np.random.SeedSequence().entropy) + if isinstance(random_seed, (bool, np.bool_)) or not isinstance( + random_seed, (int, np.integer) + ): + raise TypeError("random_seed must be a non-negative integer or None") + if random_seed < 0: + raise ValueError("random_seed must be a non-negative integer") + return int(random_seed) -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 _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), - } - - -_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. - - 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] - - -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 _what_the_rows_say_drew_them(path): - """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, - 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 - numbered = [] - with open(path, "r", encoding="utf-8") as recorded: +def _checkpoint_rows(path): + """Read validated checkpoint metadata without retaining physical results.""" + rows = [] + with open(path, encoding="utf-8") as recorded: for line in recorded: if not line.strip(): continue @@ -194,64 +87,34 @@ def _what_the_rows_say_drew_them(path): 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." + f"cannot continue {path}: a row cannot be read" ) from error - root = row.get(_SIMULATION_ROOT_KEY) if isinstance(row, dict) else None - if root is None: + index = row.get("index") if isinstance(row, dict) else None + if not isinstance(index, int) or isinstance(index, bool) or index < 0: 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." + f"cannot continue {path}: a row does not number its simulation" ) - 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: - 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), numbered - - -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, - ) - + rows.append({key: row[key] for key in ("index", "seed") if key in row}) + return rows -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 _checkpoint_seed(inputs, requested_seed): + """Recover one integer seed, rejecting mixed or damaged checkpoints.""" + recorded = [row["seed"] for row in inputs if "seed" in row] + if any( + not isinstance(seed, int) or isinstance(seed, bool) or seed < 0 + for seed in recorded + ): + raise ValueError("Cannot resume a log with an invalid seed") + seeds = set(recorded) + if len(seeds) > 1: + raise ValueError("Cannot resume more than one study in the same log") + if not seeds: + return None + seed = seeds.pop() + if requested_seed is not None and requested_seed != seed: + raise ValueError("Cannot append with a different root seed") + return seed def _refuse_logs_this_run_cannot_write( @@ -496,19 +359,19 @@ 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 + random_seed : int, 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. + sampling. Keyword-only. Default is None, which generates and prints + an integer seed that can be used to reproduce the study. - Every input row carries the root it was drawn from, so an append + Every input row carries the integer seed, 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. + Seeds must be non-negative integers. A legacy log without seeds + can be extended with a warning, without recovering its old stream. kwargs : dict Custom arguments for simulation export of the ``inputs`` file. Options are: @@ -555,22 +418,26 @@ def simulate( self._export_config = kwargs self.number_of_simulations = number_of_simulations 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 - # 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. + self.__study_seed = _study_seed(random_seed) + self._completed_indices = set() + self._index_offset = 0 _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: - # 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) + self.__continue_study(random_seed) + if ( + random_seed is None + and not self._completed_indices + and self.__has_object_seed() + ): + warnings.warn( + "MonteCarlo reseeds stochastic objects for every simulation; " + "their object seeds are ignored. Pass random_seed to reproduce the study.", + UserWarning, + stacklevel=2, + ) + print(f"Monte Carlo random_seed: {self.__study_seed}") print("Starting Monte Carlo analysis") @@ -688,11 +555,12 @@ def __run_in_serial(self): """ sim_monitor = _SimMonitor( initial_count=self._initial_sim_idx, - n_simulations=self.number_of_simulations, + n_simulations=self.number_of_simulations + self._index_offset, start_time=time(), ) + sim_monitor.skip_completed(self._completed_indices) inputs_json = "" - sim_idx = sim_monitor.count + sim_idx = self._initial_sim_idx try: # Counted from zero, as the parallel path already does: the two # used to name the same simulation 1, 2, 3 and 0, 1, 2. @@ -747,10 +615,11 @@ def __run_in_parallel(self, n_workers=None): simulation_error_event = manager.Event() sim_monitor = manager._SimMonitor( initial_count=self._initial_sim_idx, - n_simulations=self.number_of_simulations, + n_simulations=self.number_of_simulations + self._index_offset, start_time=time(), ) + sim_monitor.skip_completed(self._completed_indices) processes = [] try: @@ -787,7 +656,11 @@ def __run_in_parallel(self, n_workers=None): # An exit code cannot show a worker that left between # claiming an index and recording it. _refuse_logs_missing_a_simulation( - self.input_file, self.output_file, self.number_of_simulations + self.input_file, + self.output_file, + self.number_of_simulations, + index_offset=self._index_offset, + previous_indices=self._completed_indices, ) sim_monitor.print_final_status() @@ -898,61 +771,45 @@ def __report_a_failed_simulation(self, sim_idx, inputs_json, mutex, error_event) mutex.release() return announced - 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, 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( - 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 0 + def __continue_study(self, random_seed): + """Recover the seed and completed indices without requiring a contiguous run.""" + inputs = _checkpoint_rows(self.input_file) + outputs = _checkpoint_rows(self.output_file) + held = [row["index"] for row in inputs] + written = [row["index"] for row in outputs] if held != written: - raise ValueError( - f"cannot append to {self.input_file}: it and {self.output_file} " - 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 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 " - 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." + raise ValueError("Input and output logs do not record the same simulations") + if len(held) != len(set(held)): + raise ValueError("Cannot resume duplicate simulation indices") + recorded_seed = _checkpoint_seed(inputs, random_seed) + if recorded_seed is not None: + self.__study_seed = recorded_seed + legacy = [row for row in inputs if "seed" not in row] + if legacy: + if any("seed" in row for row in inputs[: len(legacy)]): + raise ValueError("Legacy rows must precede all seeded rows") + warnings.warn( + "Appending a legacy v1.13 study: its original random stream " + "cannot be recovered; new rows use the reported study seed.", + UserWarning, + stacklevel=3, ) - return len(held) + self._index_offset = int(all(row["index"] >= 1 for row in legacy)) + self._initial_sim_idx = self._index_offset + self._completed_indices = set(held) + + def __has_object_seed(self): + """Include nested rocket components whose seeds will also be overridden.""" + models = [self.environment, self.rocket, self.flight] + for name in getattr(self.rocket, "_POSITIONED_COLLECTIONS", ()): + models.extend(component for component, _ in getattr(self.rocket, name)) + for name in getattr(self.rocket, "_PLAIN_COLLECTIONS", ()): + models.extend(getattr(self.rocket, name)) + return any(getattr(model, "_seed", None) is not None for model in models) 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) + """Derive independent model streams from the study seed and simulation index.""" + child = np.random.SeedSequence([self.__study_seed, 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)) @@ -1063,6 +920,8 @@ def simulate_convergence( batch_size=50, parallel=False, n_workers=None, + *, + random_seed=None, ): """Run Monte Carlo simulations in batches until the confidence interval width converges within the specified tolerance or the maximum number of @@ -1084,6 +943,9 @@ def simulate_convergence( Whether to run simulations in parallel. Default is False. n_workers : int, optional The number of worker processes to use if running in parallel. Default is None. + random_seed : int, optional + Non-negative study seed passed to every batch. If omitted, an existing + study keeps its recorded seed; a new study generates and prints one. Returns ------- @@ -1126,6 +988,7 @@ def simulate_convergence( include_function_data=False, parallel=parallel, n_workers=n_workers, + random_seed=random_seed, ) self.import_outputs(self.filename.with_suffix(".outputs.txt")) @@ -1178,7 +1041,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) + inputs_dict[_SIMULATION_SEED_KEY] = self.__study_seed return ( json.dumps(inputs_dict, cls=RocketPyEncoder, **self._export_config) + "\n" ) @@ -1215,10 +1078,6 @@ def __evaluate_flight_outputs(self, flight, sim_idx): # 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" ) @@ -1373,12 +1232,9 @@ def _check_data_collector(self, data_collector): f"is written after the collectors run and cannot be " f"replaced by one." ) - if key == _SIMULATION_ROOT_KEY: + if key in ("seed", "run_root"): 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." + f"Invalid data_collector key {key!r}: reserved metadata" ) if not callable(callback): raise ValueError( @@ -1703,6 +1559,8 @@ def set_results(self): self.results = {} for result in self.outputs_log: for key, value in result.items(): + if key in _RESULT_METADATA: + continue if key in self.results: self.results[key].append(value) else: @@ -2263,24 +2121,14 @@ def _indices_a_log_holds(path): return found -def _refuse_logs_missing_a_simulation(input_file, output_file, target): - """Raise unless both logs hold every simulation the run was asked for. - - An exit code says how a worker ended, never whether the index it had - already claimed reached the logs, and the monitor counts claims rather than - rows. A worker that leaves between the two is invisible to everything else - here, so the logs themselves are what the run is judged on. - - Rows numbered past the target are left alone: an append given a smaller - target than the checkpoint already holds is an append question, not a lost - simulation. What each log holds still has to be the consecutive run it - claims to be, so its indices are required to be exactly as many as its - rows, which refuses a stray number and a hole without needing to be told - how long the checkpoint was. The two logs must also agree row for row, - since a record goes into both under one lock. Streamed rather than read - through ``_read_log_file``, which would hold every row in memory. +def _refuse_logs_missing_a_simulation( + input_file, output_file, target, index_offset=0, previous_indices=() +): + """Require paired logs covering the target and all checkpointed indices. + Reject unreadable rows, duplicates and unexpected indices; previously + completed rows beyond a smaller target remain valid. """ - wanted = set(range(target)) + wanted = set(range(index_offset, target + index_offset)) | set(previous_indices) recorded = {} for label, path in (("input", input_file), ("output", output_file)): found = _indices_a_log_holds(path) @@ -2303,7 +2151,11 @@ def _refuse_logs_missing_a_simulation(input_file, output_file, target): f"{len(missing)} of {target} simulations, the first being " f"{missing[0]}." ) - strays = sorted(held - set(range(len(found)))) + strays = sorted( + held + - set(range(index_offset, len(found) + index_offset)) + - set(previous_indices) + ) if strays: raise RuntimeError( f"The run is incomplete: the {label} log numbers a simulation " @@ -2394,8 +2246,19 @@ def __init__(self, initial_count, n_simulations, start_time): self.n_simulations = n_simulations self.start_time = start_time self.completed_count = 0 + self._pending_count = n_simulations - initial_count + self._completed_indices = set() self._claim = threading.Lock() # proxy calls run in the manager + def skip_completed(self, indices): + """Exclude checkpointed simulations from new claims and progress totals.""" + self._completed_indices = set(indices) + self._pending_count = max(0, self.n_simulations - self.count) - sum( + self.count <= index < self.n_simulations + for index in self._completed_indices + ) + self.initial_count = len(self._completed_indices) + def claim_next_index(self): """The next index to run, or None once every one has been claimed. @@ -2403,6 +2266,8 @@ def claim_next_index(self): see the last slot free and then claim an index each past the end. """ with self._claim: + while self.count in self._completed_indices: + self.count += 1 if self.count >= self.n_simulations: return None claimed = self.count @@ -2423,7 +2288,7 @@ def print_update_status(self): self.completed_count += 1 average_time = (time() - self.start_time) / self.completed_count - remaining = self.n_simulations - self.initial_count - self.completed_count + remaining = self._pending_count - self.completed_count estimated_time = int(remaining * average_time) msg = f"Iterations completed: {self.completed_count:06d}" @@ -2435,8 +2300,11 @@ def print_update_status(self): def print_final_status(self): """Prints the final status of the simulation.""" print() - msg = f"Completed {self.count - self.initial_count} iterations." - msg += f" In total, {self.count} simulations are exported.\n" + msg = f"Completed {self.completed_count} iterations." + msg += ( + f" In total, {self.initial_count + self.completed_count} " + "simulations are exported.\n" + ) msg += f"Total wall time: {time() - self.start_time:.1f} s" _SimMonitor.reprint(msg, end="\n", flush=True) diff --git a/rocketpy/stochastic/custom_sampler.py b/rocketpy/stochastic/custom_sampler.py index a29b8041d..d17562a3f 100644 --- a/rocketpy/stochastic/custom_sampler.py +++ b/rocketpy/stochastic/custom_sampler.py @@ -47,7 +47,9 @@ def reset_seed(self, seed=None): Parameters ---------- seed : int, optional - Seed for the random number generator. The default is None + Seed for the random number generator. The default is None. + Stochastic models supply a non-negative 32-bit integer, compatible + with both NumPy RandomState and Generator. Returns ------- diff --git a/rocketpy/stochastic/stochastic_flight.py b/rocketpy/stochastic/stochastic_flight.py index 79211abee..a3acb196b 100644 --- a/rocketpy/stochastic/stochastic_flight.py +++ b/rocketpy/stochastic/stochastic_flight.py @@ -64,10 +64,13 @@ def __init__( The inclination of the launch rail. heading : int, float, tuple, list, optional The heading of the launch rail. - initial_solution : tuple, list, optional + initial_solution : tuple, list, Flight, optional The initial solution of the flight. This is a tuple of 14 elements that represent the initial conditions of the flight. This attribute - can not be randomized. + can not be randomized. A Flight supplies its final state, copied + when this stochastic model is constructed. If omitted, an explicit + initial state from the base flight is preserved; rail launches + retain their randomized launch orientation. terminate_on_apogee : bool, optional Whether or not the flight should terminate on apogee. This attribute can not be randomized. @@ -78,9 +81,10 @@ def __init__( The maximum time of the flight simulation. If the flight simulation reaches this time, it will terminate. This attribute can not be randomized. """ - if terminate_on_apogee is not None: - if not isinstance(terminate_on_apogee, bool): - raise AssertionError("`terminate_on_apogee` must be a boolean") + if terminate_on_apogee is not None and not isinstance( + terminate_on_apogee, bool + ): + raise AssertionError("`terminate_on_apogee` must be a boolean") if time_overshoot is not None: if not isinstance(time_overshoot, bool): raise TypeError("`time_overshoot` must be a boolean") @@ -94,6 +98,10 @@ def __init__( heading=heading, ) + if initial_solution is None and flight.initial_derivative != flight.udot_rail1: + initial_solution = list(flight.solution[0]) + elif isinstance(initial_solution, Flight): + initial_solution = list(initial_solution.solution[-1]) self._validate_initial_solution(initial_solution) self.initial_solution = initial_solution self.terminate_on_apogee = terminate_on_apogee diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index 381e13a9b..9afa426d9 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -240,6 +240,7 @@ def _set_stochastic(self, seed=None): seed : int, optional Seed for the random number generator. """ + self._seed = seed self.__random_number_generator = np.random.default_rng(seed) # A stream of its own, derived from the same seed, for picking between # the candidate values of a list input. Kept apart from the one above so @@ -714,10 +715,8 @@ def _reset_custom_samplers(self, seed): # may reset differently, or keep state of its own. resetter = group if hasattr(group, "reset_seed") else sampler try: - resetter.reset_seed(_sampler_seed(seed, names)) + resetter.reset_seed(_sampler_seed(seed, names) & 0xFFFFFFFF) except Exception as error: - # Broad: the seed is 128 bits, which legacy RandomState refuses - # with a ValueError that does not name the sampler. raise RuntimeError( f"An error occurred in the 'reset_seed' method of the " f"CustomSampler for {', '.join(names)}" diff --git a/rocketpy/stochastic/stochastic_solid_motor.py b/rocketpy/stochastic/stochastic_solid_motor.py index 8b05f252f..0e56ed57a 100644 --- a/rocketpy/stochastic/stochastic_solid_motor.py +++ b/rocketpy/stochastic/stochastic_solid_motor.py @@ -220,4 +220,5 @@ def create_object(self): "coordinate_system_orientation" ], interpolation_method=generated_dict["interpolate"], + grains_bonded=self.obj.grains_bonded, ) diff --git a/tests/integration/simulation/test_monte_carlo_release.py b/tests/integration/simulation/test_monte_carlo_release.py new file mode 100644 index 000000000..df01d3a6d --- /dev/null +++ b/tests/integration/simulation/test_monte_carlo_release.py @@ -0,0 +1,166 @@ +"""Release regressions exercised through real Monte Carlo flights.""" + +import json + +import matplotlib.pyplot as plt +import pytest + +from rocketpy import MonteCarlo +from rocketpy.stochastic import ( + StochasticEnvironment, + StochasticFlight, + StochasticRocket, + StochasticSolidMotor, +) + + +@pytest.fixture +def release_study( + tmp_path, example_spaceport_env, calisto_robust, flight_calisto_robust +): + """Build an isolated study with a small set of scalar outputs.""" + stochastic_flight = StochasticFlight(flight_calisto_robust, max_time=1) + stochastic_flight.obj.equations_of_motion = "generalized" + stochastic_rocket = StochasticRocket(calisto_robust, mass=(15, 0.1)) + stochastic_rocket.add_motor( + StochasticSolidMotor(calisto_robust.motor), + position=[calisto_robust.motor_position], + ) + return MonteCarlo( + filename=str(tmp_path / "study"), + environment=StochasticEnvironment(example_spaceport_env), + rocket=stochastic_rocket, + flight=stochastic_flight, + export_list=["max_speed", "t_final"], + ) + + +def _rows(path): + return [json.loads(line) for line in path.read_text(encoding="utf-8").splitlines()] + + +def _write_rows(path, rows): + path.write_text("".join(json.dumps(row) + "\n" for row in rows), encoding="utf-8") + + +def test_new_run_plots_only_physical_results(release_study): + """Metadata must not reach boxplots or summary statistics.""" + release_study.simulate(2, random_seed=42, include_function_data=False) + + try: + release_study.plots.all() + assert set(release_study.results) == {"max_speed", "t_final"} + finally: + plt.close("all") + + +@pytest.mark.parametrize("parallel", [False, True]) +@pytest.mark.parametrize("target", [3, 4]) +def test_resume_fills_holes_without_repeating_completed_flights( + release_study, parallel, target +): + """A completed later index must survive recovery of an interrupted run.""" + release_study.simulate(4, random_seed=42, include_function_data=False) + expected_inputs = _rows(release_study.input_file) + expected_outputs = _rows(release_study.output_file) + for path in (release_study.input_file, release_study.output_file): + _write_rows(path, [row for row in _rows(path) if row["index"] != 1]) + + release_study.simulate( + target, append=True, parallel=parallel, n_workers=2, include_function_data=False + ) + + actual_inputs = _rows(release_study.input_file) + actual_outputs = _rows(release_study.output_file) + assert [row["index"] for row in actual_inputs] == [0, 2, 3, 1] + assert {row["index"]: row["mass"] for row in actual_inputs} == { + row["index"]: row["mass"] for row in expected_inputs + } + assert sorted(actual_outputs, key=lambda row: row["index"]) == expected_outputs + + +def test_each_input_row_records_one_integer_seed(release_study): + """The checkpoint needs the study seed, without NumPy state or digests.""" + release_study.simulate(2, random_seed=42, include_function_data=False) + + assert all(row["seed"] == 42 for row in _rows(release_study.input_file)) + assert all("run_root" not in row for row in _rows(release_study.input_file)) + assert all( + "seed" not in row and "run_root" not in row + for row in _rows(release_study.output_file) + ) + + +@pytest.mark.parametrize("component", ["environment", "motor"]) +def test_object_seed_without_study_seed_warns_and_reports_replay_seed( + release_study, capsys, component +): + """An overridden object seed must not silently suggest reproducibility.""" + model = ( + release_study.environment + if component == "environment" + else release_study.rocket.motors[0][0] + ) + model._set_stochastic(42) + + with pytest.warns(UserWarning, match="object seeds are ignored"): + release_study.simulate(1, include_function_data=False) + + seed = _rows(release_study.input_file)[0]["seed"] + assert isinstance(seed, int) + assert f"Monte Carlo random_seed: {seed}" in capsys.readouterr().out + + +@pytest.mark.parametrize("parallel", [False, True]) +def test_v113_study_can_be_extended_with_a_warning(release_study, parallel): + """Keep one-based legacy rows and append without claiming reproducibility.""" + release_study.simulate(2, random_seed=42, include_function_data=False) + for path in (release_study.input_file, release_study.output_file): + rows = _rows(path) + for row in rows: + row.pop("run_root", None) + row.pop("seed", None) + row["index"] += 1 + _write_rows(path, rows) + original = _rows(release_study.output_file) + + with pytest.warns(UserWarning, match="v1.13|legacy"): + release_study.simulate( + 4, + append=True, + random_seed=42, + parallel=parallel, + n_workers=2, + include_function_data=False, + ) + + assert _rows(release_study.output_file)[:2] == original + assert sorted(row["index"] for row in _rows(release_study.output_file)) == [ + 1, + 2, + 3, + 4, + ] + + +def test_convergence_seed_reproduces_inputs_across_batch_sizes(release_study): + """Batches must continue the same random stream.""" + release_study.simulate_convergence( + target_attribute="max_speed", + tolerance=1e-12, + max_simulations=4, + batch_size=2, + random_seed=42, + ) + expected = [row["mass"] for row in _rows(release_study.input_file)] + release_study.simulate(0, append=False, random_seed=42) + + release_study.simulate_convergence( + target_attribute="max_speed", + tolerance=1e-12, + max_simulations=4, + batch_size=4, + random_seed=42, + ) + + assert [row["mass"] for row in _rows(release_study.input_file)] == expected diff --git a/tests/unit/simulation/test_append_lineage.py b/tests/unit/simulation/test_append_lineage.py index 35f80844b..a006dd999 100644 --- a/tests/unit/simulation/test_append_lineage.py +++ b/tests/unit/simulation/test_append_lineage.py @@ -1,391 +1,148 @@ +"""Validate checkpoints before allowing Monte Carlo to append any rows.""" + import json +from types import SimpleNamespace import pytest -from rocketpy.simulation.monte_carlo import ( - MonteCarlo, - _root_digest, - _SIMULATION_ROOT_KEY, -) +from rocketpy import MonteCarlo -def _study(tmp_path, stem, models): - environment, rocket, flight = models - return MonteCarlo( - filename=str(tmp_path / stem), - environment=environment, - rocket=rocket, - flight=flight, +@pytest.fixture +def checkpoint(tmp_path): + """Create paired logs without executing a flight.""" + study = MonteCarlo( + str(tmp_path / "study"), SimpleNamespace(), SimpleNamespace(), SimpleNamespace() ) + study._MonteCarlo__study_seed = 42 + study._completed_indices = set() + study._index_offset = 0 + _write(study.input_file, [{"index": 0, "seed": 42}, {"index": 1, "seed": 42}]) + _write(study.output_file, [{"index": 0, "apogee": 10}, {"index": 1, "apogee": 20}]) + return study -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) +def _write(path, rows): + path.write_text("".join(json.dumps(row) + "\n" for row in rows), encoding="utf-8") - 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_checkpoint_recovers_the_recorded_seed(checkpoint): + """A fresh object's entropy must not replace the checkpoint's seed.""" + checkpoint._MonteCarlo__study_seed = 99 -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) + checkpoint._MonteCarlo__continue_study(None) - part = _study(tmp_path, "part", models) - part.simulate(2, append=False, random_seed=42) - part.simulate(4, append=True) + assert checkpoint._MonteCarlo__study_seed == 42 + assert checkpoint._completed_indices == {0, 1} - assert _drawn(part) == _drawn(whole) +@pytest.mark.parametrize("seed", [42, None]) +def test_checkpoint_accepts_a_matching_or_omitted_seed(checkpoint, seed): + """Explicitly repeating the seed and reading it back are equivalent.""" + checkpoint._MonteCarlo__continue_study(seed) -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) + assert checkpoint._completed_indices == {0, 1} - second = _study(tmp_path, "study", models) - second.simulate(4, append=True) - assert sorted(_drawn(second)) == [0, 1, 2, 3] +def test_checkpoint_rejects_a_different_seed(checkpoint): + """Appending cannot mix two seeded studies.""" + checkpoint._MonteCarlo__study_seed = 7 + with pytest.raises(ValueError, match="different root seed"): + checkpoint._MonteCarlo__continue_study(7) -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) +@pytest.mark.parametrize("seed", [None, True, -1, 1.5, "42", [42], {}]) +def test_checkpoint_rejects_invalid_recorded_seeds(checkpoint, seed): + """A damaged seed must be rejected before opening files for append.""" + _write( + checkpoint.input_file, [{"index": 0, "seed": seed}, {"index": 1, "seed": seed}] + ) - assert sorted(_drawn(analysis)) == [0, 1, 2, 3] + with pytest.raises(ValueError, match="invalid seed"): + checkpoint._MonteCarlo__continue_study(None) -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") +def test_checkpoint_rejects_mixed_seeds(checkpoint): + """Matching indices do not justify mixing different input streams.""" + _write(checkpoint.input_file, [{"index": 0, "seed": 42}, {"index": 1, "seed": 7}]) 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 _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) - - 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) - + checkpoint._MonteCarlo__continue_study(None) -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_checkpoint_rejects_missing_seeds_after_seeded_rows(checkpoint): + """Only an unchanged legacy prefix may omit the study seed.""" + _write(checkpoint.input_file, [{"index": 0, "seed": 42}, {"index": 1}]) + with pytest.raises(ValueError, match="Legacy rows must precede"): + checkpoint._MonteCarlo__continue_study(None) -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) +@pytest.mark.parametrize("numbers", [[0, 2], [2, 0], [1, 2]]) +def test_checkpoint_accepts_holes_and_completion_order(checkpoint, numbers): + """An interrupted parallel run may have completed later indices first.""" + _write(checkpoint.input_file, [{"index": index, "seed": 42} for index in numbers]) + _write(checkpoint.output_file, [{"index": index} for index in numbers]) - assert sorted(_drawn(analysis)) == [0, 1, 2, 3] + checkpoint._MonteCarlo__continue_study(None) + assert checkpoint._completed_indices == set(numbers) + assert checkpoint._index_offset == 0 -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) - - -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="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"} - - 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 - - -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_checkpoint_rejects_duplicate_indices(checkpoint): + """Repeated indices cannot be silently counted as separate flights.""" + for path in (checkpoint.input_file, checkpoint.output_file): + _write(path, [{"index": 0, "seed": 42}] * 2) -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") + with pytest.raises(ValueError, match="duplicate"): + checkpoint._MonteCarlo__continue_study(None) - 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] +@pytest.mark.parametrize("index", [True, 1.0, "1", None, -1]) +def test_checkpoint_rejects_invalid_indices(checkpoint, index): + """Only non-negative integer indices can identify completed simulations.""" + _write(checkpoint.input_file, [{"index": index, "seed": 42}]) + with pytest.raises(ValueError, match="does not number"): + checkpoint._MonteCarlo__continue_study(None) -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) +def test_checkpoint_rejects_mismatched_logs(checkpoint): + """A missing half of a committed record needs repair before resuming.""" + _write(checkpoint.output_file, [{"index": 0}]) + with pytest.raises(ValueError, match="same simulations"): + checkpoint._MonteCarlo__continue_study(None) -@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_checkpoint_rejects_unreadable_rows(checkpoint): + """A truncated row cannot be mistaken for an empty checkpoint.""" + checkpoint.input_file.write_text('{"index":', encoding="utf-8") + with pytest.raises(ValueError, match="cannot be read"): + checkpoint._MonteCarlo__continue_study(None) -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) +def test_blank_lines_do_not_count_as_simulations(checkpoint): + """Whitespace does not change which completed indices are skipped.""" + with checkpoint.input_file.open("a", encoding="utf-8") as log: + log.write("\n \n") - assert sorted(_drawn(analysis)) == [0, 1, 2, 3] + checkpoint._MonteCarlo__continue_study(None) + assert checkpoint._completed_indices == {0, 1} -@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) +@pytest.mark.parametrize("key", ["index", "seed", "run_root"]) +def test_collectors_cannot_overwrite_checkpoint_metadata(tmp_path, key): + """Reserved names must not silently replace a collector's output.""" + with pytest.raises(ValueError, match="Invalid"): + MonteCarlo( + str(tmp_path / "study"), + None, + None, + None, + data_collector={key: lambda flight: 1}, + ) diff --git a/tests/unit/simulation/test_monte_carlo.py b/tests/unit/simulation/test_monte_carlo.py index e9e541bf7..5adeae510 100644 --- a/tests/unit/simulation/test_monte_carlo.py +++ b/tests/unit/simulation/test_monte_carlo.py @@ -815,6 +815,9 @@ def __init__(self, filename, interrupt_after): self._initial_sim_idx = 0 self.interrupt_after = interrupt_after self.completed = 0 + self.environment = types.SimpleNamespace() + self.rocket = types.SimpleNamespace() + self.flight = types.SimpleNamespace() def _MonteCarlo__seed_this_simulation(self, sim_idx): pass @@ -826,36 +829,10 @@ def _MonteCarlo__run_single_simulation(self): return object() def _MonteCarlo__evaluate_flight_inputs(self, index): - # The root goes in because an append reads it back to check the rows - # it is continuing were drawn by the same stream; a row without it is - # what a study written before that check looks like, and is refused. - return ( - json.dumps( - { - "index": index, - mc_module._SIMULATION_ROOT_KEY: ( - mc_module._root_written_into_a_row(self._MonteCarlo__root_state) - ), - } - ) - + "\n" - ) + return json.dumps({"index": index, "seed": self._MonteCarlo__study_seed}) + "\n" def _MonteCarlo__evaluate_flight_outputs(self, flight, index): - # The outputs rows name the same root by its digest, which is what - # an append checks them against. - return ( - json.dumps( - { - "index": index, - "apogee": 1000.0 + index, - mc_module._SIMULATION_ROOT_KEY: mc_module._root_digest( - mc_module._root_written_into_a_row(self._MonteCarlo__root_state) - ), - } - ) - + "\n" - ) + return json.dumps({"index": index, "apogee": 1000.0 + index}) + "\n" def test_interrupted_serial_run_reaches_the_caller(tmp_path): @@ -1035,6 +1012,9 @@ def __init__(self, **_kwargs): def print_final_status(self): self.final_status_calls += 1 + def skip_completed(self, indices): + pass + def test_interrupted_parallel_run_signals_joins_and_reaches_the_caller( tmp_path, monkeypatch diff --git a/tests/unit/simulation/test_monte_carlo_seeding.py b/tests/unit/simulation/test_monte_carlo_seeding.py index bd1f0cfa2..132193650 100644 --- a/tests/unit/simulation/test_monte_carlo_seeding.py +++ b/tests/unit/simulation/test_monte_carlo_seeding.py @@ -6,15 +6,13 @@ import numpy as np import pytest -from rocketpy.stochastic import StochasticEnvironment from rocketpy.simulation.monte_carlo import ( MonteCarlo, - _root_seed_sequence, - _root_state_of, - _seed_of_simulation, _seed_sequence_to_int, _SimMonitor, + _study_seed, ) +from rocketpy.stochastic import StochasticEnvironment def _what_was_drawn(value): @@ -41,8 +39,7 @@ def _what_was_drawn(value): def _seed_for(index): """The seed MonteCarlo gives one index, without running the others.""" - root = _root_state_of(_root_seed_sequence(42)) - return _seed_sequence_to_int(_seed_of_simulation(root, index)) + return _seed_sequence_to_int(np.random.SeedSequence([42, index])) def _sampled_inputs(analysis): @@ -58,6 +55,7 @@ def _a_run(tmp_path, stem, models, *, parallel=False, workers=None, seed=None, c environment=environment, rocket=rocket, flight=flight, + export_list=["max_speed", "t_final"], ) analysis.simulate( number_of_simulations=count, @@ -65,81 +63,39 @@ def _a_run(tmp_path, stem, models, *, parallel=False, workers=None, seed=None, c parallel=parallel, n_workers=workers, random_seed=seed, + include_function_data=False, ) return analysis @pytest.fixture(name="models") def _models(stochastic_environment, stochastic_calisto, stochastic_flight): + stochastic_flight.max_time = 1 + stochastic_flight.obj.equations_of_motion = "generalized" 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) +@pytest.mark.parametrize("seed", [0, 42, np.int64(7), 2**128]) +def test_study_seed_accepts_non_negative_integers(seed): + """A study records a plain integer even when given a NumPy integer.""" + assert _study_seed(seed) == seed + assert isinstance(_study_seed(seed), int) - 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("seed", [True, 1.5, "42", [1, 2], np.random.SeedSequence(7)]) +def test_study_seed_rejects_non_integer_values(seed): + """Stateful generators and sequences are not integer study seeds.""" + with pytest.raises(TypeError, match="random_seed must be"): + _study_seed(seed) -@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_study_seed_rejects_negative_values(): + """Reject invalid seeds before any simulation log can be truncated.""" + with pytest.raises(ValueError, match="non-negative"): + _study_seed(-1) +@pytest.mark.slow 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) @@ -153,6 +109,7 @@ def test_an_unusable_seed_costs_the_previous_run_nothing(models, tmp_path): # ------------------------------------------------------------- what a run gives +@pytest.mark.slow 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) @@ -160,6 +117,7 @@ def test_one_seed_gives_one_set_of_inputs(models, tmp_path): assert _sampled_inputs(first) == _sampled_inputs(again) +@pytest.mark.slow 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. @@ -170,6 +128,7 @@ def test_another_seed_gives_another_set(models, tmp_path): @pytest.mark.skipif(os.cpu_count() < 4, reason="needs four workers to be four") +@pytest.mark.slow 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. @@ -217,11 +176,12 @@ def test_the_indices_of_that_run_are_not_all_one_value(example_spaceport_env): assert len(set(drawn)) == len(drawn) +@pytest.mark.slow 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) + part.simulate(4, append=True, random_seed=42, include_function_data=False) assert _sampled_inputs(part) == _sampled_inputs(whole) @@ -237,6 +197,7 @@ def _spawned_workers(): @pytest.mark.usefixtures("spawned_workers") @pytest.mark.skipif(os.cpu_count() < 4, reason="needs four workers to be four") +@pytest.mark.slow def test_an_index_keeps_its_inputs_when_the_workers_are_spawned(models, tmp_path): """The same guarantee on the start method Windows uses. @@ -273,6 +234,7 @@ def set(self): raise AssertionError("the producer reported a failure") +@pytest.mark.slow 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 @@ -284,8 +246,9 @@ def test_a_worker_loop_draws_the_study_serial_draws(models, tmp_path): environment=environment, rocket=rocket, flight=flight, + export_list=["max_speed", "t_final"], ) - worker.simulate(number_of_simulations=0, append=False, random_seed=42) + worker.simulate(0, append=False, random_seed=42, include_function_data=False) worker._MonteCarlo__sim_producer( _SimMonitor(initial_count=0, n_simulations=4, start_time=time()), diff --git a/tests/unit/simulation/test_monte_carlo_simulation_index.py b/tests/unit/simulation/test_monte_carlo_simulation_index.py index 4ac75b67f..ca63d0c46 100644 --- a/tests/unit/simulation/test_monte_carlo_simulation_index.py +++ b/tests/unit/simulation/test_monte_carlo_simulation_index.py @@ -13,15 +13,19 @@ def _indices(analysis): def _a_study(tmp_path, stem, environment, rocket, flight): + flight.max_time = 1 + flight.obj.equations_of_motion = "generalized" return MonteCarlo( filename=str(tmp_path / stem), environment=environment, rocket=rocket, flight=flight, + export_list=["max_speed", "t_final"], ) @pytest.mark.parametrize("parallel", [False, True]) +@pytest.mark.slow def test_a_run_numbers_its_simulations_from_zero( stochastic_environment, stochastic_calisto, stochastic_flight, tmp_path, parallel ): @@ -40,11 +44,13 @@ def test_a_run_numbers_its_simulations_from_zero( append=False, parallel=parallel, n_workers=2 if parallel else None, + include_function_data=False, ) assert _indices(analysis) == [0, 1, 2] +@pytest.mark.slow def test_both_modes_agree_on_what_a_simulation_is_called( stochastic_environment, stochastic_calisto, stochastic_flight, tmp_path ): @@ -59,12 +65,15 @@ def test_both_modes_agree_on_what_a_simulation_is_called( 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) + one.simulate(3, append=False, parallel=False, include_function_data=False) + other.simulate( + 3, append=False, parallel=True, n_workers=2, include_function_data=False + ) assert _indices(one) == _indices(other) +@pytest.mark.slow def test_an_appended_run_carries_on_from_the_last_index( stochastic_environment, stochastic_calisto, stochastic_flight, tmp_path ): @@ -72,12 +81,13 @@ def test_an_appended_run_carries_on_from_the_last_index( 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) + analysis.simulate(2, append=False, include_function_data=False) + analysis.simulate(4, append=True, include_function_data=False) assert _indices(analysis) == [0, 1, 2, 3] +@pytest.mark.slow def test_a_serial_failure_names_the_simulation_the_way_parallel_would( stochastic_environment, stochastic_calisto, @@ -110,7 +120,7 @@ def fails_on_the_second(self): ) with pytest.raises(RuntimeError, match="the flight would not run"): - analysis.simulate(number_of_simulations=3, append=False) + analysis.simulate(3, append=False, include_function_data=False) assert "Error on iteration 1:" in capsys.readouterr().out @@ -162,7 +172,7 @@ 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._MonteCarlo__study_seed = 42 analysis.data_collector = {"index": lambda _flight: 999} row = json.loads(analysis._MonteCarlo__evaluate_flight_outputs(None, 7)) diff --git a/tests/unit/simulation/test_monte_carlo_worker_reporting.py b/tests/unit/simulation/test_monte_carlo_worker_reporting.py index 696823a86..6f17692e2 100644 --- a/tests/unit/simulation/test_monte_carlo_worker_reporting.py +++ b/tests/unit/simulation/test_monte_carlo_worker_reporting.py @@ -86,11 +86,7 @@ def _a_worker(tmp_path, model, event=None): rocket=model, flight=model, ) - # simulate() settles the root before any worker starts, and the producer - # derives each index's seed from it, so a worker driven directly needs one. - study._MonteCarlo__root_state = mc_module._root_state_of( - mc_module._root_seed_sequence(42) - ) + study._MonteCarlo__study_seed = 42 return study, event or _ErrorEvent() diff --git a/tests/unit/stochastic/test_custom_sampler.py b/tests/unit/stochastic/test_custom_sampler.py index 286d97cd3..8694500c2 100644 --- a/tests/unit/stochastic/test_custom_sampler.py +++ b/tests/unit/stochastic/test_custom_sampler.py @@ -274,9 +274,8 @@ def test_a_sampler_that_refuses_its_seed_is_named_in_the_error(failure): assert raised.value.__cause__ is failure -def test_a_legacy_random_state_sampler_is_named_rather_than_raising_bare(): - """The concrete case, not a stand-in: RandomState really does refuse the - 128-bit seed this hands out.""" +def test_a_legacy_random_state_sampler_is_reproducible(): + """CustomSampler receives an integer within RandomState's 32-bit range.""" class LegacySampler(CustomSampler): """Built on RandomState rather than default_rng.""" @@ -285,12 +284,15 @@ def sample(self, n_samples=1): return list(self.rng.normal(size=n_samples)) def reset_seed(self, seed=None): + assert isinstance(seed, int) and 0 <= seed < 2**32 self.rng = np.random.RandomState(seed) - with pytest.raises(RuntimeError, match="mass") as raised: - StochasticModel(SimpleNamespace(mass=0.0), mass=LegacySampler()) + model = StochasticModel(SimpleNamespace(mass=0.0), seed=42, mass=LegacySampler()) + expected = next(model.dict_generator())["mass"] + + model._set_stochastic(42) - assert isinstance(raised.value.__cause__, ValueError) + assert next(model.dict_generator())["mass"] == expected class _CountingShared(_SharedPair): diff --git a/tests/unit/stochastic/test_release_configuration.py b/tests/unit/stochastic/test_release_configuration.py new file mode 100644 index 000000000..c7fee5d9a --- /dev/null +++ b/tests/unit/stochastic/test_release_configuration.py @@ -0,0 +1,64 @@ +"""Regression checks for stochastic configurations used by Monte Carlo.""" + +import numpy as np +import pytest + +from rocketpy import Flight +from rocketpy.stochastic import StochasticFlight, StochasticSolidMotor + + +def test_stochastic_flight_accepts_a_previous_flight(flight_calisto_robust): + """A continuation must use the last state of the supplied flight.""" + nominal = Flight( + flight_calisto_robust.rocket, + flight_calisto_robust.env, + rail_length=5.2, + max_time=1, + ) + stochastic = StochasticFlight( + nominal, initial_solution=nominal, max_time=nominal.t_final + 1 + ) + + continued = stochastic.create_object() + + assert continued.solution[0] == pytest.approx(nominal.solution[-1]) + + +@pytest.mark.parametrize("bonded", [False, True]) +def test_stochastic_solid_motor_preserves_grain_bonding(cesaroni_m1670, bonded): + """A deterministic grain configuration must survive stochastic wrapping.""" + cesaroni_m1670.grains_bonded = bonded + stochastic = StochasticSolidMotor(cesaroni_m1670) + + sampled = stochastic.create_object() + + assert sampled.grains_bonded is bonded + assert sampled.center_of_propellant_mass(1) == pytest.approx( + cesaroni_m1670.center_of_propellant_mass(1) + ) + + +def test_stochastic_flight_keeps_normal_rail_launch(flight_calisto_robust): + """The nominal flight's generated initial state must not bypass the rail.""" + stochastic = StochasticFlight(flight_calisto_robust) + + sampled = stochastic.create_object() + + assert sampled.out_of_rail_time > sampled.t_initial + assert np.linalg.norm(sampled.out_of_rail_state[3:6]) > 0 + + +def test_stochastic_flight_preserves_nominal_explicit_state(flight_calisto_robust): + """Wrapping a continuation must retain its initial free-flight state.""" + state = list(min(flight_calisto_robust.solution, key=lambda row: abs(row[0] - 1))) + nominal = Flight( + flight_calisto_robust.rocket, + flight_calisto_robust.env, + rail_length=5.2, + initial_solution=state, + max_time=state[0] + 1, + ) + + sampled = StochasticFlight(nominal).create_object() + + assert sampled.solution[0] == pytest.approx(state) From b1e75cbe8808c22bc9848d5110e480f50ddb8e84 Mon Sep 17 00:00:00 2001 From: Gui-FernandesBR Date: Sat, 7 Nov 2026 01:16:35 -0300 Subject: [PATCH 2/2] BUG: address review of the Monte Carlo release fixes Keep the base flight's terminate_on_apogee and use_udot_rail2 in StochasticFlight and MonteCarlo, accept NumPy scalars in initial_solution, and stop a checkpointed log's length from hiding a stray index. Seeds a study derives for itself no longer count as object seeds, so a second study does not warn about them. Drop docs/TODO.md and fold the changelog's breaking-changes section into Changed. #1209 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01XDW574nZTvpQsbhf7kAaFz --- CHANGELOG.md | 9 +-- docs/TODO.md | 5 -- docs/user/stochastic.rst | 16 ++--- rocketpy/simulation/monte_carlo.py | 23 +++++-- rocketpy/stochastic/stochastic_flight.py | 20 ++++-- rocketpy/stochastic/stochastic_model.py | 4 +- rocketpy/stochastic/stochastic_rocket.py | 2 + .../simulation/test_monte_carlo_release.py | 10 +++ tests/unit/simulation/test_monte_carlo.py | 2 + .../test_monte_carlo_run_completeness.py | 21 ++++++ .../simulation/test_monte_carlo_seeding.py | 55 ++++++++++++++++ .../stochastic/test_release_configuration.py | 64 +++++++++++++++++++ 12 files changed, 199 insertions(+), 32 deletions(-) delete mode 100644 docs/TODO.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cefb0991..9394ab151 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,11 +30,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/). -### Breaking changes - -- New serial Monte Carlo studies use indices `0..n-1` instead of `1..n`, matching parallel studies. Legacy logs retain their existing numbering when extended. [#1187](https://github.com/RocketPy-Team/RocketPy/pull/1187) -- Monte Carlo reseeds stochastic objects for every simulation in both execution modes. Pass a non-negative integer `random_seed` to reproduce a study; individual object seeds are overridden, and fixed-seed results from v1.13 change. New input rows record one integer `seed`, while output rows contain only the index and requested results. Metadata is excluded from statistical results and plots. [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) - ### Added - ENH: Accept `random_seed` in `MonteCarlo.simulate_convergence`, including across batches. [#1198](https://github.com/RocketPy-Team/RocketPy/pull/1198) [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) @@ -56,7 +51,7 @@ Attention: The newest changes should be on top --> ### Changed -- ENH: Number new serial and parallel Monte Carlo runs from zero. Legacy checkpoints retain their numbering when extended, with a warning that their original random draws cannot be reproduced. [#1187](https://github.com/RocketPy-Team/RocketPy/pull/1187) [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) +- ENH: Number new Monte Carlo runs from zero in both execution modes, as the parallel path already did; serial runs used `1..n`, so their `index` shifts by one. Every simulation is now reseeded from one study `random_seed` (a non-negative integer), so individual object seeds are ignored and fixed-seed results from v1.13 change. New input rows record that `seed`; metadata is kept out of results, statistics and plots. Legacy checkpoints keep their numbering when extended, with a warning that their original draws cannot be reproduced. [#1187](https://github.com/RocketPy-Team/RocketPy/pull/1187) [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) - ENH: Compute the rocket static margin lazily [#1135](https://github.com/RocketPy-Team/RocketPy/pull/1135) [#780](https://github.com/RocketPy-Team/RocketPy/issues/780) - DOC: Tighten the comments that came with the sampler seed groups [#1154](https://github.com/RocketPy-Team/RocketPy/pull/1154) - CI: make the Gemini PR reviewer actually review [#1140](https://github.com/RocketPy-Team/RocketPy/pull/1140) @@ -67,7 +62,7 @@ Attention: The newest changes should be on top --> ### Fixed -- BUG: Keep checkpoint metadata out of Monte Carlo plots and statistics, recover missing simulation indices without repeating completed flights, and warn when appending v1.13 logs. Preserve explicit initial flight states and solid-motor grain bonding in stochastic models. [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) +- BUG: Keep checkpoint metadata out of Monte Carlo plots and statistics, recover missing simulation indices without repeating completed flights, and warn when appending v1.13 logs. Preserve explicit initial flight states, `terminate_on_apogee`, `use_udot_rail2` and solid-motor grain bonding in stochastic models. [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) - BUG: A `Flight` continued from another `Flight` object now records `t_initial`, so a rocket carrying sensors or controllers no longer raises `AttributeError` on that path. [#920](https://github.com/RocketPy-Team/RocketPy/pull/920) - BUG: Correct the gravity sign an `Accelerometer` applies when `consider_gravity=True`. The gravitational field was added to the inertial acceleration instead of subtracted from it, so the sensor reported the negative of the proper acceleration along the vertical: one at rest read -g rather than +g. Recorded accelerometer data taken with `consider_gravity=True` changes sign in that term. [#1175](https://github.com/RocketPy-Team/RocketPy/pull/1175) - BUG: Report a Monte Carlo worker that fails instead of hanging or passing for a finished run [#1182](https://github.com/RocketPy-Team/RocketPy/pull/1182) diff --git a/docs/TODO.md b/docs/TODO.md deleted file mode 100644 index c4ab6fe9e..000000000 --- a/docs/TODO.md +++ /dev/null @@ -1,5 +0,0 @@ -# Monte Carlo follow-ups - -The v1.14 fixes tracked in [#1209](https://github.com/RocketPy-Team/RocketPy/issues/1209) -retain the current worker orchestration. A pool rewrite and validation once per -study remain deferred to v1.15, as agreed in the release review. diff --git a/docs/user/stochastic.rst b/docs/user/stochastic.rst index edb054d35..19a9f54ff 100644 --- a/docs/user/stochastic.rst +++ b/docs/user/stochastic.rst @@ -353,6 +353,13 @@ draws under a fixed seed. The rocket's own inputs, such as ``mass`` and individual stochastic models are overridden; a new study without ``random_seed`` warns when these models have already been seeded. + An index fixes the draw, not the object it is drawn around. Where the note + above says a value follows its deterministic object, moving that object + between runs still moves what is sampled, however the seed was set. A + ``CustomSampler`` that draws from the process-global ``numpy.random`` + rather than from the generator it is handed sits outside all of this, as + :ref:`custom_sampler` warns. + ``simulate_convergence(random_seed=...)`` uses the same study seed for every batch. Changing the batch size does not change the inputs of a given index. @@ -372,15 +379,6 @@ seed. Existing one-based serial numbering is preserved; new studies start at zero. Checkpoint fields (``index``, ``seed`` and the earlier ``run_root`` field) are excluded from ``results``, summary statistics and plots. -.. note:: - - An index fixes the draw, not the object it is drawn around. Where the note - above says a value follows its deterministic object, moving that object - between runs still moves what is sampled, however the seed was set. A - ``CustomSampler`` that draws from the process-global ``numpy.random`` - rather than from the generator it is handed sits outside all of this, as - :ref:`custom_sampler` warns. - Conclusion ---------- diff --git a/rocketpy/simulation/monte_carlo.py b/rocketpy/simulation/monte_carlo.py index 6cae9d030..0fadf1c1d 100644 --- a/rocketpy/simulation/monte_carlo.py +++ b/rocketpy/simulation/monte_carlo.py @@ -805,15 +805,22 @@ def __has_object_seed(self): models.extend(component for component, _ in getattr(self.rocket, name)) for name in getattr(self.rocket, "_PLAIN_COLLECTIONS", ()): models.extend(getattr(self.rocket, name)) - return any(getattr(model, "_seed", None) is not None for model in models) + return any( + getattr(model, "_configured_seed", None) is not None for model in models + ) def __seed_this_simulation(self, sim_idx): """Derive independent model streams from the study seed and simulation index.""" child = np.random.SeedSequence([self.__study_seed, 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)) + for model, model_seed in ( + (self.environment, environment), + (self.rocket, rocket), + (self.flight, flight), + ): + model._set_stochastic(_seed_sequence_to_int(model_seed)) + # Derived from the study seed, so not one the caller configured. + model._configured_seed = None def __run_single_simulation(self): """Runs a single simulation and returns the inputs and outputs. @@ -834,6 +841,7 @@ def __run_single_simulation(self): heading=flight_inputs["heading"], initial_solution=self.flight.initial_solution, terminate_on_apogee=self.flight.terminate_on_apogee, + use_udot_rail2=self.flight.obj.use_udot_rail2, time_overshoot=self.flight.time_overshoot, # The rest of what StochasticFlight.create_object passes. Left out # here, a run ignored the max_time, tolerances, solver, equations of @@ -2151,16 +2159,19 @@ def _refuse_logs_missing_a_simulation( f"{len(missing)} of {target} simulations, the first being " f"{missing[0]}." ) + # With a checkpoint, only the target and the checkpoint are valid; the + # log's own length must not widen the range, or it hides a stray. + upper = target if previous_indices else max(target, len(found)) strays = sorted( held - - set(range(index_offset, len(found) + index_offset)) + - set(range(index_offset, upper + index_offset)) - set(previous_indices) ) if strays: raise RuntimeError( f"The run is incomplete: the {label} log numbers a simulation " f"{strays[0]}, past the {len(found)} it holds, so what it " - f"records is not one run of consecutive simulations." + f"records is not the run, or the checkpoint, that was asked for." ) if recorded["input"] != recorded["output"]: diff --git a/rocketpy/stochastic/stochastic_flight.py b/rocketpy/stochastic/stochastic_flight.py index a3acb196b..5fc5df3f1 100644 --- a/rocketpy/stochastic/stochastic_flight.py +++ b/rocketpy/stochastic/stochastic_flight.py @@ -1,5 +1,9 @@ """Defines the StochasticFlight class.""" +from numbers import Real + +import numpy as np + from rocketpy.simulation import Flight from .stochastic_model import StochasticModel @@ -28,7 +32,7 @@ class StochasticFlight(StochasticModel): be randomized. terminate_on_apogee : bool Whether or not the flight should terminate on apogee. This attribute - can not be randomized. + can not be randomized. If omitted, the base flight's value is kept. time_overshoot : bool If False, the simulation will run at the time step defined by the controller sampling rate. Be aware that this will make the simulation run much slower. @@ -73,7 +77,7 @@ def __init__( retain their randomized launch orientation. terminate_on_apogee : bool, optional Whether or not the flight should terminate on apogee. This attribute - can not be randomized. + can not be randomized. If omitted, the base flight's value is kept. time_overshoot : bool If False, the simulation will run at the time step defined by the controller sampling rate. Be aware that this will make the simulation run much slower. @@ -104,7 +108,11 @@ def __init__( initial_solution = list(initial_solution.solution[-1]) self._validate_initial_solution(initial_solution) self.initial_solution = initial_solution - self.terminate_on_apogee = terminate_on_apogee + self.terminate_on_apogee = ( + flight.terminate_on_apogee + if terminate_on_apogee is None + else terminate_on_apogee + ) if max_time is None: self.max_time = flight.max_time else: @@ -124,7 +132,10 @@ def _validate_initial_solution(self, initial_solution): "vx_init, vy_init, vz_init, e0_init, e1_init, e2_init, " "e3_init, w1Init, w2Init, w3Init" ) - if not all(isinstance(i, (int, float)) for i in initial_solution): + if not all( + isinstance(i, Real) and not isinstance(i, (bool, np.bool_)) + for i in initial_solution + ): raise AssertionError( "`initial_solution` must be a tuple of numbers" ) @@ -172,6 +183,7 @@ def create_object(self): heading=generated_dict["heading"], initial_solution=self.initial_solution, terminate_on_apogee=self.terminate_on_apogee, + use_udot_rail2=self.obj.use_udot_rail2, max_time=self.max_time, max_time_step=self.obj.max_time_step, min_time_step=self.obj.min_time_step, diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index 9afa426d9..7fd3e8bf6 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -240,7 +240,9 @@ def _set_stochastic(self, seed=None): seed : int, optional Seed for the random number generator. """ - self._seed = seed + # What the caller chose. A parent that derives this seed for a child, or + # a Monte Carlo study, clears it again: a derived seed is not a choice. + self._configured_seed = seed self.__random_number_generator = np.random.default_rng(seed) # A stream of its own, derived from the same seed, for picking between # the candidate values of a list input. Kept apart from the one above so diff --git a/rocketpy/stochastic/stochastic_rocket.py b/rocketpy/stochastic/stochastic_rocket.py index 03154eabe..67831f49b 100644 --- a/rocketpy/stochastic/stochastic_rocket.py +++ b/rocketpy/stochastic/stochastic_rocket.py @@ -212,6 +212,7 @@ def _set_stochastic(self, seed=None): component._set_stochastic( _seed_sequence_to_int(roots[name].spawn(1)[0]) ) + component._configured_seed = None def __reset_components(self, components, root): """Creates a new Components whose stochastic structures @@ -236,6 +237,7 @@ def __reset_components(self, components, root): for stochastic_obj, _ in components: stochastic_obj_position_info = self.__components_map[stochastic_obj] stochastic_obj._set_stochastic(_seed_sequence_to_int(root.spawn(1)[0])) + stochastic_obj._configured_seed = None new_components.add( stochastic_obj, self._validate_position(stochastic_obj, stochastic_obj_position_info), diff --git a/tests/integration/simulation/test_monte_carlo_release.py b/tests/integration/simulation/test_monte_carlo_release.py index df01d3a6d..4892ae162 100644 --- a/tests/integration/simulation/test_monte_carlo_release.py +++ b/tests/integration/simulation/test_monte_carlo_release.py @@ -111,6 +111,16 @@ def test_object_seed_without_study_seed_warns_and_reports_replay_seed( assert f"Monte Carlo random_seed: {seed}" in capsys.readouterr().out +def test_a_second_study_does_not_warn_about_seeds_the_first_one_set( + release_study, recwarn +): + """Seeds a study derives for itself are not seeds the caller configured.""" + release_study.simulate(1, random_seed=42, include_function_data=False) + release_study.simulate(1, append=False, include_function_data=False) + + assert not [w for w in recwarn if "object seeds are ignored" in str(w.message)] + + @pytest.mark.parametrize("parallel", [False, True]) def test_v113_study_can_be_extended_with_a_warning(release_study, parallel): """Keep one-based legacy rows and append without claiming reproducibility.""" diff --git a/tests/unit/simulation/test_monte_carlo.py b/tests/unit/simulation/test_monte_carlo.py index 5adeae510..ca9a0f390 100644 --- a/tests/unit/simulation/test_monte_carlo.py +++ b/tests/unit/simulation/test_monte_carlo.py @@ -608,6 +608,7 @@ def test_a_monte_carlo_flight_keeps_the_configuration_it_was_given(monkeypatch): equations_of_motion="solid_propulsion", ode_solver="RK23", simulation_mode="native", + use_udot_rail2=True, ) stochastic_flight = types.SimpleNamespace( obj=base, @@ -637,6 +638,7 @@ def test_a_monte_carlo_flight_keeps_the_configuration_it_was_given(monkeypatch): assert flight.ode_solver == "RK23" assert flight.equations_of_motion == "solid_propulsion" assert flight.simulation_mode == "native" + assert flight.use_udot_rail2 is True assert flight.name == "named" # and the single draw is what the flight is actually built from assert (flight.rail_length, flight.inclination, flight.heading) == ( diff --git a/tests/unit/simulation/test_monte_carlo_run_completeness.py b/tests/unit/simulation/test_monte_carlo_run_completeness.py index d98f51390..bcea826bc 100644 --- a/tests/unit/simulation/test_monte_carlo_run_completeness.py +++ b/tests/unit/simulation/test_monte_carlo_run_completeness.py @@ -120,6 +120,27 @@ def test_a_stray_index_past_the_rows_is_refused(tmp_path): _refuse_logs_missing_a_simulation(inputs, outputs, 2) +def test_a_stray_is_not_hidden_by_how_long_a_checkpointed_log_is(tmp_path): + """With a checkpoint, the rows held do not widen what a run may number.""" + # The log is longer than the target because of the checkpoint, and sizing + # the range off that length let index 3 pass for a simulation of this run. + rows = [_row(0), _row(1), _row(3), _row(10)] + inputs = _a_log(tmp_path, "gap.inputs.txt", rows) + outputs = _a_log(tmp_path, "gap.outputs.txt", rows) + + with pytest.raises(RuntimeError, match="numbers a simulation 3"): + _refuse_logs_missing_a_simulation(inputs, outputs, 2, previous_indices={0, 10}) + + +def test_a_checkpoint_with_gaps_is_accepted_once_the_gaps_are_filled(tmp_path): + """The control: indices of the target plus the checkpoint's own are valid.""" + rows = [_row(0), _row(10), _row(1)] + inputs = _a_log(tmp_path, "fill.inputs.txt", rows) + outputs = _a_log(tmp_path, "fill.outputs.txt", rows) + + _refuse_logs_missing_a_simulation(inputs, outputs, 2, previous_indices={0, 10}) + + def test_the_order_a_parallel_run_finishes_in_is_not_a_hole(tmp_path): """Rows arrive in completion order, which is not sorted and not wrong.""" rows = [_row(1), _row(0), _row(2)] diff --git a/tests/unit/simulation/test_monte_carlo_seeding.py b/tests/unit/simulation/test_monte_carlo_seeding.py index 132193650..9694afe3d 100644 --- a/tests/unit/simulation/test_monte_carlo_seeding.py +++ b/tests/unit/simulation/test_monte_carlo_seeding.py @@ -257,3 +257,58 @@ def test_a_worker_loop_draws_the_study_serial_draws(models, tmp_path): ) assert _sampled_inputs(worker) == _sampled_inputs(serial) + + +def _an_unrun_study(tmp_path, models): + environment, rocket, flight = models + return MonteCarlo( + filename=str(tmp_path / "unrun"), + environment=environment, + rocket=rocket, + flight=flight, + ) + + +def test_a_study_that_nobody_seeded_has_no_object_seed(models, tmp_path): + """The control: fresh models carry no seed the caller chose.""" + assert not _an_unrun_study(tmp_path, models)._MonteCarlo__has_object_seed() + + +@pytest.mark.parametrize("which", ["environment", "rocket", "flight", "motor"]) +def test_a_seed_the_caller_set_counts_as_an_object_seed(models, tmp_path, which): + """A seed on any model a run overrides is one the user has to be told of.""" + environment, rocket, flight = models + seeded = { + "environment": environment, + "rocket": rocket, + "flight": flight, + "motor": rocket.motors[0][0], + }[which] + seeded._set_stochastic(42) + + assert _an_unrun_study(tmp_path, models)._MonteCarlo__has_object_seed() + + +def test_the_seeds_a_study_derives_are_not_object_seeds(models, tmp_path): + """Reseeding for one simulation must not look like the caller's choice.""" + # Otherwise a second study without random_seed warned that "object seeds + # are ignored" about seeds the first study had put there itself. + study = _an_unrun_study(tmp_path, models) + study._MonteCarlo__study_seed = 42 + + study._MonteCarlo__seed_this_simulation(3) + + assert not study._MonteCarlo__has_object_seed() + + +def test_a_seed_set_on_a_component_is_cleared_by_the_reseed(models, tmp_path): + """A component's own seed is replaced by one derived from the rocket's.""" + _, rocket, _ = models + rocket.motors[0][0]._set_stochastic(7) + study = _an_unrun_study(tmp_path, models) + study._MonteCarlo__study_seed = 42 + assert study._MonteCarlo__has_object_seed() + + study._MonteCarlo__seed_this_simulation(0) + + assert not study._MonteCarlo__has_object_seed() diff --git a/tests/unit/stochastic/test_release_configuration.py b/tests/unit/stochastic/test_release_configuration.py index c7fee5d9a..b70199a5f 100644 --- a/tests/unit/stochastic/test_release_configuration.py +++ b/tests/unit/stochastic/test_release_configuration.py @@ -62,3 +62,67 @@ def test_stochastic_flight_preserves_nominal_explicit_state(flight_calisto_robus sampled = StochasticFlight(nominal).create_object() assert sampled.solution[0] == pytest.approx(state) + + +def test_stochastic_flight_keeps_nominal_terminate_on_apogee(flight_calisto_robust): + """An omitted ``terminate_on_apogee`` must not reset the nominal flight's.""" + nominal = Flight( + flight_calisto_robust.rocket, + flight_calisto_robust.env, + rail_length=5.2, + terminate_on_apogee=True, + max_time=1, + ) + + kept = StochasticFlight(nominal) + overridden = StochasticFlight(nominal, terminate_on_apogee=False) + + assert kept.create_object().terminate_on_apogee is True + assert overridden.create_object().terminate_on_apogee is False + + +def test_stochastic_flight_keeps_nominal_tip_off_phase(flight_calisto_robust): + """The opt-in single rail button phase must survive stochastic wrapping.""" + nominal = Flight( + flight_calisto_robust.rocket, + flight_calisto_robust.env, + rail_length=5.2, + use_udot_rail2=True, + max_time=1, + ) + + sampled = StochasticFlight(nominal).create_object() + + assert sampled.use_udot_rail2 is True + + +@pytest.mark.parametrize("scalar", [np.float32, np.float64, np.int64]) +def test_stochastic_flight_accepts_numpy_scalars_in_initial_solution( + flight_calisto_robust, scalar +): + """A state sliced out of a NumPy array is a state, not a type error.""" + state = [scalar(value) for value in flight_calisto_robust.solution[0]] + + stochastic = StochasticFlight(flight_calisto_robust, initial_solution=state) + + assert stochastic.initial_solution == state + + +@pytest.mark.parametrize("flag", [True, np.bool_(True)]) +def test_stochastic_flight_rejects_booleans_in_initial_solution( + flight_calisto_robust, flag +): + """A boolean compares equal to a number and is still not one.""" + state = list(flight_calisto_robust.solution[0]) + state[3] = flag + + with pytest.raises(AssertionError, match="tuple of numbers"): + StochasticFlight(flight_calisto_robust, initial_solution=state) + + +def test_stochastic_flight_rejects_a_non_boolean_terminate_on_apogee( + flight_calisto_robust, +): + """``terminate_on_apogee`` has to be a real boolean when it is given.""" + with pytest.raises(AssertionError, match="terminate_on_apogee"): + StochasticFlight(flight_calisto_robust, terminate_on_apogee=1)