From b116e84dc131ac6dcc150790999bde7d0c290716 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sun, 16 Aug 2026 01:13:52 +0800 Subject: [PATCH 01/16] BUG: sample around the nominal a stochastic model was built with _set_stochastic re-validates every declared input, and validation reads the nominal off the wrapped object. create_object writes the sampled value back onto that same object on purpose, so re-reading it on a reseed took one simulation's output as the next one's nominal: a wind factor compounded 10 -> 8.576 -> 7.355 -> 6.308 under a single fixed seed, and a plain scalar spec drifted the same way. Read the nominal once and keep it. Containers are copied on the way in, so writing through the wrapped object cannot reach it either. A component position arrives through an injected getter, reads an attribute nothing writes back to, and shares one name across every component, so those are read live rather than cached. Extracted from #1054. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- docs/user/stochastic.rst | 8 ++ rocketpy/stochastic/stochastic_model.py | 44 +++++- .../unit/stochastic/test_stochastic_model.py | 133 +++++++++++++++++- 3 files changed, 179 insertions(+), 6 deletions(-) diff --git a/docs/user/stochastic.rst b/docs/user/stochastic.rst index 6e3376236..c3cc947c9 100644 --- a/docs/user/stochastic.rst +++ b/docs/user/stochastic.rst @@ -130,6 +130,14 @@ passed in a few different ways: A ``CustomSampler`` given for this argument has to yield a whole outline per sample, since what it returns replaces the outline instead of perturbing it. +.. note:: + Where the nominal value comes from the deterministic object, it is read when + the ``Stochastic`` object is built and kept from then on. Changing the + deterministic object afterwards does not move what is sampled around, and + neither does a ``MonteCarlo`` run: ``create_object`` writes each sampled + value back onto that object, so re-reading it would take one simulation's + output as the next one's nominal. + .. note:: In statistics, the terms "Normal" and "Gaussian" refer to the same type of \ distribution. This distribution is commonly used and is the default for the \ diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index d42fb76c5..f21b45cd0 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -3,6 +3,8 @@ Stochastic classes. """ +from copy import deepcopy + import numpy as np from rocketpy.mathutils.function import Function @@ -11,6 +13,20 @@ from ..tools import get_distribution +def _snapshot_of(value): + """A nominal that writing through the wrapped object cannot reach. + + Containers are copied, since ``obj.shape_points[:] = ...`` would otherwise + move the value this is meant to hold still. Anything else is kept by + reference: nothing here mutates a ``Function`` or a motor in place. + """ + if isinstance(value, np.ndarray): + return value.copy() + if isinstance(value, (list, dict, set)): + return deepcopy(value) + return value + + def _names_as_spawn_key(input_names): """Encode names into spawn-key words that no other set of names produces. @@ -122,8 +138,26 @@ def __init__(self, obj, seed=None, **kwargs): self.obj = obj self.last_rnd_dict = {} self.__stochastic_dict = kwargs + self.__nominal_values = {} self._set_stochastic(seed) + def _nominal(self, input_name, getter=getattr): + """``self.obj``'s value for ``input_name`` as it was when built. + + Kept because ``create_object`` writes the sampled value back onto + ``self.obj``, so re-reading it on a reseed takes one simulation's output + as the next one's nominal. An injected ``getter`` reads a component's + own attribute, which nothing writes back to, and every component's + position arrives under the one name, so those are not cached. + """ + if getter is not getattr: + return getter(self.obj, input_name) + if input_name not in self.__nominal_values: + self.__nominal_values[input_name] = _snapshot_of( + getattr(self.obj, input_name) + ) + return self.__nominal_values[input_name] + def _declare_stochastic_input(self, input_name, input_value): """Declare an input that an ``add_*`` method installs after ``__init__``. @@ -186,7 +220,7 @@ def _set_stochastic(self, seed=None): "or a custom sampler" ) else: - attr_value = [getattr(self.obj, input_name)] + attr_value = [self._nominal(input_name)] setattr(self, input_name, attr_value) def __repr__(self): @@ -321,7 +355,7 @@ def _validate_tuple_length_two(self, input_name, input_value, getattr=getattr): # object passed. dist_func = get_distribution(input_value[1], self.__random_number_generator) return ( - self._nominal_value(input_name, getattr(self.obj, input_name)), + self._nominal_value(input_name, self._nominal(input_name, getattr)), input_value[0], dist_func, ) @@ -397,7 +431,7 @@ def _validate_list(self, input_name, input_value, getattr=getattr): # pylint: d If the input is not in a valid format. """ if not input_value: - return [getattr(self.obj, input_name)] + return [self._nominal(input_name, getattr)] else: return input_value @@ -423,7 +457,7 @@ def _validate_scalar(self, input_name, input_value, getattr=getattr): # pylint: distribution function). """ return ( - self._nominal_value(input_name, getattr(self.obj, input_name)), + self._nominal_value(input_name, self._nominal(input_name, getattr)), input_value, get_distribution("normal", self.__random_number_generator), ) @@ -450,7 +484,7 @@ def _validate_factors(self, input_name, input_value): If the input is not in a valid format. """ attribute_name = input_name.replace("_factor", "") - setattr(self, f"_{attribute_name}", getattr(self.obj, attribute_name)) + setattr(self, f"_{attribute_name}", self._nominal(attribute_name)) if isinstance(input_value, tuple): return self._validate_tuple_factor(input_name, input_value) diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index 8bb360c48..2263dc5b8 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -1,6 +1,10 @@ +import numpy as np import pytest -from rocketpy.stochastic import StochasticFreeFormFins +from rocketpy import Environment +from rocketpy.mathutils.function import Function +from rocketpy.stochastic import StochasticEnvironment, StochasticFreeFormFins +from rocketpy.stochastic.stochastic_model import _snapshot_of @pytest.mark.parametrize( @@ -50,3 +54,130 @@ def spans(seed): # Both candidates must stay reachable, or the assertions above would also # hold for a generator that always returned the same one. assert set(spans(7)) == {0.1, 0.12} + + +def _windy_environment(): + environment = Environment() + environment.set_atmospheric_model(type="standard_atmosphere") + environment.wind_velocity_x = 10.0 + return environment + + +def _effective_wind_x(environment): + """The wind the Environment would actually fly with.""" + wind = environment.wind_velocity_x + return float(wind(0)) if callable(wind) else float(wind) + + +def test_a_factor_does_not_compound_across_reseeds(): + """Reseeding with the same seed has to give the same inputs. + + ``StochasticEnvironment.create_object`` writes the sampled value back onto + the Environment rather than building a copy, so re-reading the nominal from + it compounded: 10 -> 8.576 -> 7.355 -> 6.308, each the last one multiplied + by the same factor again. + """ + stochastic = StochasticEnvironment( + environment=_windy_environment(), wind_velocity_x_factor=(1.0, 0.1) + ) + + winds = [] + for _ in range(4): + stochastic._set_stochastic(12345) + winds.append(_effective_wind_x(stochastic.create_object())) + + assert len(set(winds)) == 1, f"the same seed drifted across reseeds: {winds}" + + +def test_a_seed_gives_the_same_input_whatever_was_sampled_before_it(): + """Caching the nominal per model, not per seed, is what this pins. + + Reseeding to 103 has to give what it gives on a fresh model, whether or not + 102 and 101 ran first. A cache keyed by the seed would satisfy the test + above and still fail here, because each new seed would re-read a nominal + the previous ``create_object`` had already moved. + """ + + def wind_after(seeds): + stochastic = StochasticEnvironment( + environment=_windy_environment(), wind_velocity_x_factor=(1.0, 0.1) + ) + wind = None + for seed in seeds: + stochastic._set_stochastic(seed) + wind = _effective_wind_x(stochastic.create_object()) + return wind + + assert wind_after([101, 102, 103]) == wind_after([103]) + + +def test_a_scalar_nominal_does_not_drift_across_reseeds(): + """Not only the factors. + + ``_validate_scalar`` and the ``(std, "distribution")`` tuple both take their + nominal from the wrapped object, so a plain scalar spec drifts the same way + a factor compounds. + """ + environment = Environment() + environment.set_atmospheric_model(type="standard_atmosphere") + stochastic = StochasticEnvironment(environment=environment, elevation=100.0) + + elevations = [] + for _ in range(4): + stochastic._set_stochastic(2024) + elevations.append(float(stochastic.create_object().elevation)) + + assert len(set(elevations)) == 1, f"the nominal elevation drifted: {elevations}" + + +def test_the_nominal_is_the_one_the_model_was_built_with(example_plain_env): + """Snapshot semantics, stated once and pinned here. + + A model samples around what the wrapped object held when it was built, so a + later change to that object deliberately does not move what is sampled + around. That is the same rule the drift above depends on. + """ + example_plain_env.elevation = 1000 + # A scalar is a spread around the object's own value, so this is the form + # that reads the nominal. A tuple carries its own centre and would not. + model = StochasticEnvironment(environment=example_plain_env, elevation=5) + + model._set_stochastic(4242) + around_first = model.elevation[0] + + example_plain_env.elevation = 9000 + model._set_stochastic(4242) + + assert model.elevation[0] == around_first == 1000, ( + "the model followed the object instead of the value it was built with" + ) + + +def test_a_mutable_nominal_survives_a_write_through_the_object( + calisto_free_form_fins, +): + """Holding the object itself would let ``obj.shape_points[:] = ...`` reach + the nominal and move a value the model is supposed to sample around.""" + stochastic = StochasticFreeFormFins( + free_form_fins=calisto_free_form_fins, shape_points=0.001 + ) + stochastic._set_stochastic(7) + expected = np.array(stochastic.shape_points[0], copy=True) + + stochastic.obj.shape_points[:] = [(9.9, 9.9)] * len(stochastic.obj.shape_points) + stochastic._set_stochastic(7) + + assert np.array_equal(stochastic.shape_points[0], expected) + + +def test_the_snapshot_keeps_by_reference_what_it_does_not_copy(): + """Only containers are copied, so the rule can be stated as it behaves.""" + array = np.array([1.0, 2.0]) + listed = [[1.0], [2.0]] + function = Function(lambda x: x) + + assert _snapshot_of(array) is not array + assert _snapshot_of(listed) is not listed + assert _snapshot_of(listed)[0] is not listed[0] # deep, not shallow + assert _snapshot_of(function) is function + assert _snapshot_of(3.0) == 3.0 From f24514b6323e622dcbff3e73cdc01a9b9896001e Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sun, 16 Aug 2026 03:07:06 +0800 Subject: [PATCH 02/16] BUG: keep the nominal out of reach of what is generated from it Copying on the way in was not enough. _nominal handed back the kept object itself, and on the empty-spec path that one object reached the model attribute, last_rnd_dict and the FreeFormFins create_object returns, so a write through any of them moved what the next reseed sampled around. _snapshot_of stopped at a tuple as well, which left an array inside an airfoil pair shared with the object it came from. Copy on the way out too, and recurse through the built-in containers. The documented contract now names the four cases that stay outside it: an input added after construction, a component position, an ensemble wind factor, and anything that is not an array or a built-in container. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- docs/user/stochastic.rst | 25 ++++- rocketpy/stochastic/stochastic_model.py | 36 ++++--- .../unit/stochastic/test_stochastic_model.py | 100 +++++++++++++++++- 3 files changed, 140 insertions(+), 21 deletions(-) diff --git a/docs/user/stochastic.rst b/docs/user/stochastic.rst index c3cc947c9..2843e0269 100644 --- a/docs/user/stochastic.rst +++ b/docs/user/stochastic.rst @@ -132,11 +132,26 @@ passed in a few different ways: .. note:: Where the nominal value comes from the deterministic object, it is read when - the ``Stochastic`` object is built and kept from then on. Changing the - deterministic object afterwards does not move what is sampled around, and - neither does a ``MonteCarlo`` run: ``create_object`` writes each sampled - value back onto that object, so re-reading it would take one simulation's - output as the next one's nominal. + that input is configured and kept from then on, so changing the deterministic + object afterwards does not move what is sampled around. Neither does a + ``MonteCarlo`` run: ``create_object`` writes each sampled value back onto + that object, and re-reading it would take one simulation's output as the + next one's nominal. Arrays and the built-in containers are copied on the way + in and on the way out, so writing through a generated object does not reach + the kept value either. + + Four things sit outside that rule on purpose: + + - an input installed by an ``add_*`` method, an eccentricity for instance, is + read when it is added rather than when the object is built; + - a component's position is read from its own component on every reset, since + each of them arrives under the one name ``position``; + - an ensemble wind factor scales the selected member's own profile, because + ``select_ensemble_member`` rebuilds the wind and the value from before it + belongs to whichever member was loaded then; + - anything that is not an array or a built-in container, a ``Function`` or a + callable among them, is kept by reference and follows the object it came + from. .. note:: In statistics, the terms "Normal" and "Gaussian" refer to the same type of \ diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index f21b45cd0..6821a324b 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -3,8 +3,6 @@ Stochastic classes. """ -from copy import deepcopy - import numpy as np from rocketpy.mathutils.function import Function @@ -14,16 +12,24 @@ def _snapshot_of(value): - """A nominal that writing through the wrapped object cannot reach. + """Returns a copy of value that a later write cannot reach. - Containers are copied, since ``obj.shape_points[:] = ...`` would otherwise - move the value this is meant to hold still. Anything else is kept by - reference: nothing here mutates a ``Function`` or a motor in place. + Arrays and the built-in containers are copied entry by entry, since an array + inside an ``airfoil`` tuple would otherwise stay shared. Anything else is + returned as it is. """ if isinstance(value, np.ndarray): return value.copy() - if isinstance(value, (list, dict, set)): - return deepcopy(value) + if isinstance(value, list): + return [_snapshot_of(item) for item in value] + if isinstance(value, tuple): + entries = [_snapshot_of(item) for item in value] + # A namedtuple takes its fields positionally. + return type(value)(*entries) if hasattr(value, "_fields") else tuple(entries) + if isinstance(value, set): + return {_snapshot_of(item) for item in value} + if isinstance(value, dict): + return {key: _snapshot_of(item) for key, item in value.items()} return value @@ -142,13 +148,13 @@ def __init__(self, obj, seed=None, **kwargs): self._set_stochastic(seed) def _nominal(self, input_name, getter=getattr): - """``self.obj``'s value for ``input_name`` as it was when built. + """Returns what ``self.obj`` held for ``input_name`` when it was + configured. - Kept because ``create_object`` writes the sampled value back onto - ``self.obj``, so re-reading it on a reseed takes one simulation's output - as the next one's nominal. An injected ``getter`` reads a component's - own attribute, which nothing writes back to, and every component's - position arrives under the one name, so those are not cached. + Kept and copied both ways, because ``create_object`` writes sampled + values back onto that object and what this returns reaches + ``last_rnd_dict``. A position arrives through a ``getter`` and is read + live, since every component uses this one name. """ if getter is not getattr: return getter(self.obj, input_name) @@ -156,7 +162,7 @@ def _nominal(self, input_name, getter=getattr): self.__nominal_values[input_name] = _snapshot_of( getattr(self.obj, input_name) ) - return self.__nominal_values[input_name] + return _snapshot_of(self.__nominal_values[input_name]) def _declare_stochastic_input(self, input_name, input_value): """Declare an input that an ``add_*`` method installs after ``__init__``. diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index 2263dc5b8..56ff8cc3e 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -3,6 +3,7 @@ from rocketpy import Environment from rocketpy.mathutils.function import Function +from rocketpy.rocket.aero_surface import FreeFormFins from rocketpy.stochastic import StochasticEnvironment, StochasticFreeFormFins from rocketpy.stochastic.stochastic_model import _snapshot_of @@ -170,8 +171,69 @@ def test_a_mutable_nominal_survives_a_write_through_the_object( assert np.array_equal(stochastic.shape_points[0], expected) +def test_a_generated_object_cannot_move_the_kept_nominal(calisto_free_form_fins): + """The kept value has to stay private, not only be copied on the way in. + + An empty spec keeps the object's own outline, and that one object reached + the model attribute, ``last_rnd_dict`` and the fins ``create_object`` + returns, so a write through any of them moved the next reseed. + """ + expected = [tuple(point) for point in calisto_free_form_fins.shape_points] + stochastic = StochasticFreeFormFins( + free_form_fins=calisto_free_form_fins, shape_points=None + ) + + generated = stochastic.create_object() + generated.shape_points[1] = (9.9, 9.9) + stochastic._set_stochastic(7) + + assert [tuple(point) for point in stochastic.shape_points[0]] == expected + + +def test_writing_through_the_model_attribute_cannot_move_it_either(): + """The same, from the other public surface. + + ``numpy.asarray(value, dtype=float)`` hands back what it was given when that + is already a float array, so the fin is built from one here. + """ + fins = FreeFormFins( + n=4, + shape_points=np.array( + [(0, 0), (0.08, 0.1), (0.12, 0.1), (0.12, 0)], dtype=float + ), + rocket_radius=0.0635, + ) + stochastic = StochasticFreeFormFins(free_form_fins=fins, shape_points=0.001) + stochastic._set_stochastic(7) + expected = np.array(stochastic.shape_points[0], copy=True) + + stochastic.shape_points[0][1] = (9.9, 9.9) + stochastic._set_stochastic(7) + + assert np.array_equal(stochastic.shape_points[0], expected) + + +def test_a_spread_and_distribution_tuple_does_not_drift(example_plain_env): + """The ``(std, "distribution")`` form takes its centre from the object too. + + The scalar test above goes through ``_validate_scalar`` and this through + ``_validate_tuple_length_two``, so one says nothing about the other. + """ + example_plain_env.elevation = 100.0 + stochastic = StochasticEnvironment( + environment=example_plain_env, elevation=(5.0, "normal") + ) + + elevations = [] + for _ in range(4): + stochastic._set_stochastic(2024) + elevations.append(float(stochastic.create_object().elevation)) + + assert len(set(elevations)) == 1, f"the centre drifted: {elevations}" + + def test_the_snapshot_keeps_by_reference_what_it_does_not_copy(): - """Only containers are copied, so the rule can be stated as it behaves.""" + """Only built-in containers are copied, so the rule behaves as stated.""" array = np.array([1.0, 2.0]) listed = [[1.0], [2.0]] function = Function(lambda x: x) @@ -180,4 +242,40 @@ def test_the_snapshot_keeps_by_reference_what_it_does_not_copy(): assert _snapshot_of(listed) is not listed assert _snapshot_of(listed)[0] is not listed[0] # deep, not shallow assert _snapshot_of(function) is function + assert _snapshot_of({"a": [1.0]})["a"] is not None assert _snapshot_of(3.0) == 3.0 + + +def test_two_components_do_not_share_one_position_nominal(stochastic_calisto): + """Component positions are read live, through an injected getter. + + Every one of them arrives under the name ``position``, so keeping them the + way the other inputs are kept would hand the second component the first + one's place. The report test notices the getter going missing, but only + because reading ``position`` off the rocket raises; it would not notice a + key that quietly collides. + """ + stochastic_calisto._set_stochastic(5) + + places = {} + for component, position in stochastic_calisto.aerodynamic_surfaces: + nominal = position[0] + places[type(component).__name__] = float(getattr(nominal, "z", nominal)) + + assert len(places) > 1, "need more than one surface for this to say anything" + assert len(set(places.values())) == len(places), places + + +def test_the_snapshot_reaches_a_mutable_nested_in_a_tuple(): + """An ``airfoil`` is ``(source, unit)`` and the source may be an array, so + stopping at the tuple would leave that array shared with the object it came + from. A ``Function`` nested the same way still travels by reference. + """ + source = np.array([[0.0, 0.0], [1.0, 1.0]]) + function = Function(lambda x: x) + + copied = _snapshot_of((source, "degrees")) + source[0, 1] = 99.0 + + assert copied[0][0, 1] == 0.0 + assert _snapshot_of((function, "degrees"))[0] is function From 5cfc390fd6b3b0f156e16aa475ccc93eb3678cf2 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sun, 16 Aug 2026 03:19:37 +0800 Subject: [PATCH 03/16] TST: pin when a late input is captured, and the spread-tuple path An add_* input is configured after __init__, so its nominal is read then. The new test writes the rocket's eccentricity before add_cp_eccentricity and again after it, and only the first one may reach the draw. The (std, distribution) form now runs its own seed histories rather than repeating one seed, which is what a cache keyed by the seed instead of by the model actually fails. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- .../unit/stochastic/test_stochastic_model.py | 53 ++++++++++++++----- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index 56ff8cc3e..268bba055 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -4,7 +4,11 @@ from rocketpy import Environment from rocketpy.mathutils.function import Function from rocketpy.rocket.aero_surface import FreeFormFins -from rocketpy.stochastic import StochasticEnvironment, StochasticFreeFormFins +from rocketpy.stochastic import ( + StochasticEnvironment, + StochasticFreeFormFins, + StochasticRocket, +) from rocketpy.stochastic.stochastic_model import _snapshot_of @@ -213,23 +217,48 @@ def test_writing_through_the_model_attribute_cannot_move_it_either(): assert np.array_equal(stochastic.shape_points[0], expected) -def test_a_spread_and_distribution_tuple_does_not_drift(example_plain_env): +def test_a_spread_and_distribution_tuple_does_not_drift(): """The ``(std, "distribution")`` form takes its centre from the object too. The scalar test above goes through ``_validate_scalar`` and this through - ``_validate_tuple_length_two``, so one says nothing about the other. + ``_validate_tuple_length_two``, so one says nothing about the other. Each + run gets its own Environment, since ``create_object`` writes onto it. """ - example_plain_env.elevation = 100.0 - stochastic = StochasticEnvironment( - environment=example_plain_env, elevation=(5.0, "normal") - ) - elevations = [] - for _ in range(4): - stochastic._set_stochastic(2024) - elevations.append(float(stochastic.create_object().elevation)) + def elevation_after(seeds): + environment = Environment() + environment.set_atmospheric_model(type="standard_atmosphere") + environment.elevation = 100.0 + stochastic = StochasticEnvironment( + environment=environment, elevation=(5.0, "normal") + ) + drawn = None + for seed in seeds: + stochastic._set_stochastic(seed) + drawn = float(stochastic.create_object().elevation) + return drawn + + assert elevation_after([2024, 2024]) == elevation_after([2024]) + assert elevation_after([7, 11, 2024]) == elevation_after([2024]) + + +def test_an_input_added_after_the_model_takes_its_nominal_then(calisto): + """An ``add_*`` input is configured after ``__init__``, so it is read then. + + A scalar spec centres on the object's own value, and ``add_cp_eccentricity`` + is the first thing to ask for it, so what the rocket holds at that moment is + what gets kept. + """ + stochastic = StochasticRocket(rocket=calisto, radius=0.0127 / 2) + + calisto.cp_eccentricity_x = 0.5 + stochastic.add_cp_eccentricity(x=0.001) + calisto.cp_eccentricity_x = 9.0 + + stochastic._set_stochastic(11) + drawn = float(next(stochastic.dict_generator())["cp_eccentricity_x"]) - assert len(set(elevations)) == 1, f"the centre drifted: {elevations}" + assert abs(drawn - 0.5) < 0.05, f"centred on {drawn}, not on the add-time 0.5" def test_the_snapshot_keeps_by_reference_what_it_does_not_copy(): From ce43a48a74263e2c4aa6eb724b1bc8c7e2f6613c Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sun, 16 Aug 2026 03:34:38 +0800 Subject: [PATCH 04/16] TST: pin the Function nominal as a boundary, not a footnote The documented exception said a Function is held as it was given. Nothing enforced it, so closing the hole later would have gone unnoticed and the documentation would have quietly become wrong. Measured: set_source on the rocket's drag curve moves the drawn value from 0.377 to 0.890, and deepcopy of that curve costs 6 microseconds. Cost is not the reason to leave it. _snapshot_of cannot raise today, and deepcopying whatever a user passed, on a path that runs on every reseed, would make it able to. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- .../unit/stochastic/test_stochastic_model.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index 268bba055..9ee628e1d 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -275,6 +275,28 @@ def test_the_snapshot_keeps_by_reference_what_it_does_not_copy(): assert _snapshot_of(3.0) == 3.0 +def test_a_function_nominal_is_held_as_it_was_given(calisto): + """The documented exception, pinned rather than only written down. + + A ``Function`` is mutable through its own API, so ``set_source`` on the + rocket's drag curve does move the baseline. Copying it would mean + ``deepcopy`` of whatever a user passed, on a path that runs on every reseed + and today cannot fail, which is not a trade this change should make. + """ + stochastic = StochasticRocket( + rocket=calisto, radius=0.0127 / 2, power_off_drag_factor=(1.0, 0.1) + ) + stochastic._set_stochastic(4242) + before = float(stochastic.create_object().power_off_drag(0.5)) + + calisto.power_off_drag.set_source(lambda mach: 0.9) + stochastic._set_stochastic(4242) + after = float(stochastic.create_object().power_off_drag(0.5)) + + assert 0.3 < before < 0.5, before + assert 0.7 < after < 1.1, after + + def test_two_components_do_not_share_one_position_nominal(stochastic_calisto): """Component positions are read live, through an injected getter. From 9a1cd810abf4864adae139715223a2184a2d1ef2 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sun, 16 Aug 2026 04:22:19 +0800 Subject: [PATCH 05/16] BUG: read the nominal again when a late input is configured again Keeping the nominal gave the second add_cp_eccentricity nothing to replace, so it went on sampling around the value the rocket held at the first call: 0.5 where 0.8 was asked for. Reproducible, and around the wrong centre, which is harder to notice than a value that moves. Late configuration drops the kept nominal before validation reads one, and puts it back if validation raises, so a refused call leaves the previous configuration standing. Only the reconfiguration path. Passing None still leaves the earlier declaration in place, which is develop's behaviour and not this branch's. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/stochastic/stochastic_model.py | 21 +++++++++++ rocketpy/stochastic/stochastic_rocket.py | 28 +++++++++------ .../unit/stochastic/test_stochastic_model.py | 36 +++++++++++++++++++ 3 files changed, 75 insertions(+), 10 deletions(-) diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index 6821a324b..70ad6f3e0 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -82,6 +82,9 @@ def _sampler_seed(seed, input_names): return sum(int(word) << (32 * position) for position, word in enumerate(words)) +_MISSING = object() + + # TODO: Stop using assert in production code. Use exceptions instead. # TODO: Each validation method should have a test case. @@ -164,6 +167,24 @@ def _nominal(self, input_name, getter=getattr): ) return _snapshot_of(self.__nominal_values[input_name]) + def _reconfigure_stochastic_input(self, input_name, input_value, validate): + """Configures a late input again, reading its nominal as it is now. + + The kept nominal is what ``configured`` means, so replacing the input + has to drop it before validation reads one. A validation that raises + leaves the previous configuration standing. + """ + kept = self.__nominal_values.pop(input_name, _MISSING) + try: + validated = validate() + except BaseException: + self.__nominal_values.pop(input_name, None) + if kept is not _MISSING: + self.__nominal_values[input_name] = kept + raise + self._declare_stochastic_input(input_name, input_value) + return validated + def _declare_stochastic_input(self, input_name, input_value): """Declare an input that an ``add_*`` method installs after ``__init__``. diff --git a/rocketpy/stochastic/stochastic_rocket.py b/rocketpy/stochastic/stochastic_rocket.py index 65cfb5ebe..fbbefa7e5 100644 --- a/rocketpy/stochastic/stochastic_rocket.py +++ b/rocketpy/stochastic/stochastic_rocket.py @@ -456,10 +456,16 @@ def add_cp_eccentricity(self, x=None, y=None): self : StochasticRocket Object of the StochasticRocket class. """ - self.cp_eccentricity_x = self._validate_eccentricity("cp_eccentricity_x", x) - self._declare_stochastic_input("cp_eccentricity_x", x) - self.cp_eccentricity_y = self._validate_eccentricity("cp_eccentricity_y", y) - self._declare_stochastic_input("cp_eccentricity_y", y) + self.cp_eccentricity_x = self._reconfigure_stochastic_input( + "cp_eccentricity_x", + x, + lambda: self._validate_eccentricity("cp_eccentricity_x", x), + ) + self.cp_eccentricity_y = self._reconfigure_stochastic_input( + "cp_eccentricity_y", + y, + lambda: self._validate_eccentricity("cp_eccentricity_y", y), + ) return self def add_thrust_eccentricity(self, x=None, y=None): @@ -484,14 +490,16 @@ def add_thrust_eccentricity(self, x=None, y=None): self : StochasticRocket Object of the StochasticRocket class. """ - self.thrust_eccentricity_x = self._validate_eccentricity( - "thrust_eccentricity_x", x + self.thrust_eccentricity_x = self._reconfigure_stochastic_input( + "thrust_eccentricity_x", + x, + lambda: self._validate_eccentricity("thrust_eccentricity_x", x), ) - self._declare_stochastic_input("thrust_eccentricity_x", x) - self.thrust_eccentricity_y = self._validate_eccentricity( - "thrust_eccentricity_y", y + self.thrust_eccentricity_y = self._reconfigure_stochastic_input( + "thrust_eccentricity_y", + y, + lambda: self._validate_eccentricity("thrust_eccentricity_y", y), ) - self._declare_stochastic_input("thrust_eccentricity_y", y) return self def _validate_eccentricity(self, eccentricity, position): diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index 9ee628e1d..7bf3208fd 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -330,3 +330,39 @@ def test_the_snapshot_reaches_a_mutable_nested_in_a_tuple(): assert copied[0][0, 1] == 0.0 assert _snapshot_of((function, "degrees"))[0] is function + + +def test_configuring_a_late_input_again_reads_the_nominal_again(calisto): + """``configured`` has to mean the second call as well as the first. + + The kept nominal had no replacement path, so a second + ``add_cp_eccentricity`` went on sampling around the value the rocket held + at the first one. Reproducible, and around the wrong centre. + """ + stochastic = StochasticRocket(rocket=calisto, radius=0.0127 / 2) + + calisto.cp_eccentricity_x = 0.5 + stochastic.add_cp_eccentricity(x=0.001) + calisto.cp_eccentricity_x = 0.8 + stochastic.add_cp_eccentricity(x=0.001) + + assert stochastic.cp_eccentricity_x[0] == 0.8 + + stochastic._set_stochastic(11) + + assert stochastic.cp_eccentricity_x[0] == 0.8 + + +def test_a_refused_reconfiguration_leaves_the_previous_one(calisto): + """Dropping the kept nominal before validation must not outlive a failure.""" + calisto.cp_eccentricity_x = 0.5 + stochastic = StochasticRocket(rocket=calisto, radius=0.0127 / 2) + stochastic.add_cp_eccentricity(x=0.001) + + calisto.cp_eccentricity_x = 0.8 + with pytest.raises(AssertionError): + stochastic.add_cp_eccentricity(x=object()) + + stochastic._set_stochastic(11) + + assert stochastic.cp_eccentricity_x[0] == 0.5 From a259fb76133d2184d386df35bcaa3da7a9e1182b Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sun, 16 Aug 2026 04:41:45 +0800 Subject: [PATCH 06/16] DOC: say what the snapshot does not do Measured on the current implementation: a cycle recurses until Python stops it, two references to one list come back as two lists, and the elements of an object-dtype array stay shared. None of those reach a supported nominal, but the docstring read like a general deep copy and should not. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/stochastic/stochastic_model.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index 70ad6f3e0..bbfce7d6f 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -14,9 +14,10 @@ def _snapshot_of(value): """Returns a copy of value that a later write cannot reach. - Arrays and the built-in containers are copied entry by entry, since an array - inside an ``airfoil`` tuple would otherwise stay shared. Anything else is - returned as it is. + Numeric arrays and the built-in containers are copied entry by entry, since + an array inside an ``airfoil`` tuple would otherwise stay shared. This is + not a general deep copy: anything else is returned as it is, shared + structure is not rebuilt, and a cycle recurses until Python stops it. """ if isinstance(value, np.ndarray): return value.copy() From ea9136f9d51c2cc8dff874903769a29e8ff51294 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sun, 16 Aug 2026 07:53:17 +0800 Subject: [PATCH 07/16] BUG: take a late input away when it is configured to None None is a configuration too. The nominal was refreshed but the earlier distribution stayed declared, so the next reseed validated it again and drew an uncertainty the caller had asked to remove. Filed as #1171 while the removal lived elsewhere; it belongs in the replacement helper this branch added, so it is here rather than in a second PR that owns the other half of one state transition. None still means an axis that was never given, and removing what was never declared stays a no-op. Both meanings have a test. The snapshot test asserted a dict entry was not None, which held whether or not anything had been copied, and no test reached the set branch at all. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- docs/user/stochastic.rst | 11 ++--- rocketpy/stochastic/stochastic_model.py | 8 +++- .../unit/stochastic/test_stochastic_model.py | 43 ++++++++++++++++++- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/docs/user/stochastic.rst b/docs/user/stochastic.rst index 2843e0269..77e06b0a2 100644 --- a/docs/user/stochastic.rst +++ b/docs/user/stochastic.rst @@ -134,11 +134,12 @@ passed in a few different ways: Where the nominal value comes from the deterministic object, it is read when that input is configured and kept from then on, so changing the deterministic object afterwards does not move what is sampled around. Neither does a - ``MonteCarlo`` run: ``create_object`` writes each sampled value back onto - that object, and re-reading it would take one simulation's output as the - next one's nominal. Arrays and the built-in containers are copied on the way - in and on the way out, so writing through a generated object does not reach - the kept value either. + ``MonteCarlo`` run: some ``create_object`` paths write the sampled value + back onto the object they were given rather than building a copy, and + re-reading it would take one simulation's output as the next one's nominal. + Arrays and the built-in containers are copied on the way in and on the way + out, so writing through a generated object does not reach the kept value + either. Four things sit outside that rule on purpose: diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index bbfce7d6f..babfd776e 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -173,7 +173,8 @@ def _reconfigure_stochastic_input(self, input_name, input_value, validate): The kept nominal is what ``configured`` means, so replacing the input has to drop it before validation reads one. A validation that raises - leaves the previous configuration standing. + leaves the previous configuration standing, and ``None`` takes away + whatever was declared before rather than leaving it to be drawn again. """ kept = self.__nominal_values.pop(input_name, _MISSING) try: @@ -183,7 +184,10 @@ def _reconfigure_stochastic_input(self, input_name, input_value, validate): if kept is not _MISSING: self.__nominal_values[input_name] = kept raise - self._declare_stochastic_input(input_name, input_value) + if input_value is None: + self.__stochastic_dict.pop(input_name, None) + else: + self.__stochastic_dict[input_name] = input_value return validated def _declare_stochastic_input(self, input_name, input_value): diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index 7bf3208fd..5cbfd7260 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -135,7 +135,7 @@ def test_a_scalar_nominal_does_not_drift_across_reseeds(): assert len(set(elevations)) == 1, f"the nominal elevation drifted: {elevations}" -def test_the_nominal_is_the_one_the_model_was_built_with(example_plain_env): +def test_the_nominal_is_the_one_the_input_was_configured_with(example_plain_env): """Snapshot semantics, stated once and pinned here. A model samples around what the wrapped object held when it was built, so a @@ -267,11 +267,17 @@ def test_the_snapshot_keeps_by_reference_what_it_does_not_copy(): listed = [[1.0], [2.0]] function = Function(lambda x: x) + mapped = {"a": [1.0]} + grouped = {("a", 1), ("b", 2)} + assert _snapshot_of(array) is not array assert _snapshot_of(listed) is not listed assert _snapshot_of(listed)[0] is not listed[0] # deep, not shallow + assert _snapshot_of(mapped) is not mapped + assert _snapshot_of(mapped)["a"] is not mapped["a"] + assert _snapshot_of(grouped) is not grouped + assert _snapshot_of(grouped) == grouped assert _snapshot_of(function) is function - assert _snapshot_of({"a": [1.0]})["a"] is not None assert _snapshot_of(3.0) == 3.0 @@ -366,3 +372,36 @@ def test_a_refused_reconfiguration_leaves_the_previous_one(calisto): stochastic._set_stochastic(11) assert stochastic.cp_eccentricity_x[0] == 0.5 + + +def test_taking_a_late_input_away_removes_what_it_declared(calisto): + """``None`` is a configuration too, and it has to survive a reseed. + + The nominal was refreshed but the earlier distribution stayed declared, so + the next ``_set_stochastic`` validated it again and drew an uncertainty the + caller had asked to remove. + """ + stochastic = StochasticRocket(rocket=calisto, radius=0.0127 / 2) + stochastic.add_cp_eccentricity(x=0.001) + + calisto.cp_eccentricity_x = 0.8 + stochastic.add_cp_eccentricity(x=None) + stochastic._set_stochastic(11) + + assert "cp_eccentricity_x" not in next(stochastic.dict_generator()) + assert stochastic.cp_eccentricity_x == [0.8] + + +def test_a_half_that_was_never_given_is_not_declared(calisto): + """The control for the one above. + + ``None`` also means an axis the caller never mentioned, and removing what + was never there has to stay a no-op rather than an error. + """ + stochastic = StochasticRocket(rocket=calisto, radius=0.0127 / 2) + stochastic.add_cp_eccentricity(x=0.001) + + generated = next(stochastic.dict_generator()) + + assert "cp_eccentricity_x" in generated + assert "cp_eccentricity_y" not in generated From 798c0c1d37405307b5f65cf13e195fcf687e7937 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sun, 16 Aug 2026 07:57:24 +0800 Subject: [PATCH 08/16] BUG: replace a pair of late inputs together or not at all add_cp_eccentricity takes x and y in one call, so a y that will not validate left x already replaced and declared. Validation happens for the whole group before anything is committed now. The test gives only y first, so x is undeclared going in and a partial commit shows up as an eccentricity the caller never successfully asked for. Asserting the nominal alone did not catch it, since x's nominal was restored either way. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/stochastic/stochastic_model.py | 40 +++++++++++-------- rocketpy/stochastic/stochastic_rocket.py | 28 +++++-------- .../unit/stochastic/test_stochastic_model.py | 24 +++++++++++ 3 files changed, 57 insertions(+), 35 deletions(-) diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index babfd776e..56025be9f 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -168,26 +168,32 @@ def _nominal(self, input_name, getter=getattr): ) return _snapshot_of(self.__nominal_values[input_name]) - def _reconfigure_stochastic_input(self, input_name, input_value, validate): - """Configures a late input again, reading its nominal as it is now. - - The kept nominal is what ``configured`` means, so replacing the input - has to drop it before validation reads one. A validation that raises - leaves the previous configuration standing, and ``None`` takes away - whatever was declared before rather than leaving it to be drawn again. - """ - kept = self.__nominal_values.pop(input_name, _MISSING) + def _reconfigure_stochastic_inputs(self, inputs, validate): + """Configures late inputs again, all of them or none of them. + + The kept nominal is what ``configured`` means, so replacing an input + has to drop it before validation reads one. Whatever a caller passes + together is replaced together, since ``add_cp_eccentricity`` takes x + and y in one call and a y that will not validate must not leave x + already replaced. ``None`` takes away what was declared before. + """ + kept = { + name: self.__nominal_values.pop(name) + for name, _ in inputs + if name in self.__nominal_values + } try: - validated = validate() + validated = [validate(name, value) for name, value in inputs] except BaseException: - self.__nominal_values.pop(input_name, None) - if kept is not _MISSING: - self.__nominal_values[input_name] = kept + for name, _ in inputs: + self.__nominal_values.pop(name, None) + self.__nominal_values.update(kept) raise - if input_value is None: - self.__stochastic_dict.pop(input_name, None) - else: - self.__stochastic_dict[input_name] = input_value + for name, value in inputs: + if value is None: + self.__stochastic_dict.pop(name, None) + else: + self.__stochastic_dict[name] = value return validated def _declare_stochastic_input(self, input_name, input_value): diff --git a/rocketpy/stochastic/stochastic_rocket.py b/rocketpy/stochastic/stochastic_rocket.py index fbbefa7e5..257bf9a9d 100644 --- a/rocketpy/stochastic/stochastic_rocket.py +++ b/rocketpy/stochastic/stochastic_rocket.py @@ -456,15 +456,11 @@ def add_cp_eccentricity(self, x=None, y=None): self : StochasticRocket Object of the StochasticRocket class. """ - self.cp_eccentricity_x = self._reconfigure_stochastic_input( - "cp_eccentricity_x", - x, - lambda: self._validate_eccentricity("cp_eccentricity_x", x), - ) - self.cp_eccentricity_y = self._reconfigure_stochastic_input( - "cp_eccentricity_y", - y, - lambda: self._validate_eccentricity("cp_eccentricity_y", y), + self.cp_eccentricity_x, self.cp_eccentricity_y = ( + self._reconfigure_stochastic_inputs( + (("cp_eccentricity_x", x), ("cp_eccentricity_y", y)), + self._validate_eccentricity, + ) ) return self @@ -490,15 +486,11 @@ def add_thrust_eccentricity(self, x=None, y=None): self : StochasticRocket Object of the StochasticRocket class. """ - self.thrust_eccentricity_x = self._reconfigure_stochastic_input( - "thrust_eccentricity_x", - x, - lambda: self._validate_eccentricity("thrust_eccentricity_x", x), - ) - self.thrust_eccentricity_y = self._reconfigure_stochastic_input( - "thrust_eccentricity_y", - y, - lambda: self._validate_eccentricity("thrust_eccentricity_y", y), + self.thrust_eccentricity_x, self.thrust_eccentricity_y = ( + self._reconfigure_stochastic_inputs( + (("thrust_eccentricity_x", x), ("thrust_eccentricity_y", y)), + self._validate_eccentricity, + ) ) return self diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index 5cbfd7260..61bd58765 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -405,3 +405,27 @@ def test_a_half_that_was_never_given_is_not_declared(calisto): assert "cp_eccentricity_x" in generated assert "cp_eccentricity_y" not in generated + + +def test_a_pair_of_late_inputs_is_replaced_together(calisto): + """``add_cp_eccentricity`` takes x and y in one call, so a y that will not + validate must leave x exactly as it was, declaration included. + + Only y is given first, so x is undeclared going in and a partial commit + shows up as an eccentricity the caller never successfully asked for. + """ + calisto.cp_eccentricity_x = 0.5 + calisto.cp_eccentricity_y = 0.6 + stochastic = StochasticRocket(rocket=calisto, radius=0.0127 / 2) + stochastic.add_cp_eccentricity(y=0.002) + + calisto.cp_eccentricity_x = 0.8 + with pytest.raises(AssertionError): + stochastic.add_cp_eccentricity(x=0.001, y=object()) + + stochastic._set_stochastic(11) + generated = next(stochastic.dict_generator()) + + assert "cp_eccentricity_x" not in generated + assert generated["cp_eccentricity_y"] is not None + assert stochastic.cp_eccentricity_y[0] == 0.6 From 83882952440d44ee8a2e215dee7338401041e8ef Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sun, 16 Aug 2026 09:10:37 +0800 Subject: [PATCH 09/16] BUG: give every draw its own copy of a mutable value Copying on the way out of the kept nominal was not the last boundary. The list branch handed back the candidate itself, which for an empty spec is the model's own working value, and FreeFormFins keeps shape_points by reference. Writing through the first generated fins reached the second ones: first = stochastic.create_object() first.shape_points[1] = (9.9, 9.9) second = stochastic.create_object() # (9.9, 9.9) as well No reseed in between, which is how create_object is documented to be used and how a serial Monte Carlo runs it. last_rnd_dict was the same dictionary the values were built from, so it moved with them too. It records what was drawn now, which matters because a Monte Carlo writes it out after the flight rather than before. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/stochastic/stochastic_model.py | 11 +++-- rocketpy/stochastic/stochastic_rocket.py | 4 +- .../unit/stochastic/test_stochastic_model.py | 46 +++++++++++++++++-- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index 56025be9f..23855d982 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -177,6 +177,7 @@ def _reconfigure_stochastic_inputs(self, inputs, validate): and y in one call and a y that will not validate must not leave x already replaced. ``None`` takes away what was declared before. """ + inputs = tuple(inputs) kept = { name: self.__nominal_values.pop(name) for name, _ in inputs @@ -284,8 +285,10 @@ def _choose(self, values): One of the candidates, or ``values`` itself when there are none. """ if len(values) == 0: - return values - return values[self.__choice_generator.integers(len(values))] + return _snapshot_of(values) + # Copied, because what this returns is handed to the object being built + # and would otherwise be the candidate the next draw picks again. + return _snapshot_of(values[self.__choice_generator.integers(len(values))]) def _nominal_value(self, input_name, value): """Return the nominal value of an input as the distribution needs it. @@ -805,7 +808,9 @@ def dict_generator(self): raise RuntimeError( f"An error occurred in the 'sample' method of {arg} CustomSampler" ) from e - self.last_rnd_dict = generated_dict + # A record of what was drawn, not a window onto what was built from + # it: the object handed the values can be written through afterwards. + self.last_rnd_dict = _snapshot_of(generated_dict) yield generated_dict # pylint: disable=too-many-statements diff --git a/rocketpy/stochastic/stochastic_rocket.py b/rocketpy/stochastic/stochastic_rocket.py index 257bf9a9d..51e80a1cb 100644 --- a/rocketpy/stochastic/stochastic_rocket.py +++ b/rocketpy/stochastic/stochastic_rocket.py @@ -31,7 +31,7 @@ StochasticTail, StochasticTrapezoidalFins, ) -from .stochastic_model import StochasticModel +from .stochastic_model import StochasticModel, _snapshot_of from .stochastic_parachute import StochasticParachute from .stochastic_solid_motor import StochasticSolidMotor @@ -682,7 +682,7 @@ def dict_generator(self): generated_dict["rail_buttons"] = [] generated_dict["air_brakes"] = [] generated_dict["parachutes"] = [] - self.last_rnd_dict = generated_dict + self.last_rnd_dict = _snapshot_of(generated_dict) yield generated_dict def _create_motor(self, component_stochastic_motor): diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index 61bd58765..18c2ed296 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -138,9 +138,9 @@ def test_a_scalar_nominal_does_not_drift_across_reseeds(): def test_the_nominal_is_the_one_the_input_was_configured_with(example_plain_env): """Snapshot semantics, stated once and pinned here. - A model samples around what the wrapped object held when it was built, so a - later change to that object deliberately does not move what is sampled - around. That is the same rule the drift above depends on. + A model samples around what the wrapped object held when the input was + configured, so a later change to that object deliberately does not move what + is sampled around. That is the same rule the drift above depends on. """ example_plain_env.elevation = 1000 # A scalar is a spread around the object's own value, so this is the form @@ -154,7 +154,7 @@ def test_the_nominal_is_the_one_the_input_was_configured_with(example_plain_env) model._set_stochastic(4242) assert model.elevation[0] == around_first == 1000, ( - "the model followed the object instead of the value it was built with" + "the model followed the object instead of the value it was configured with" ) @@ -429,3 +429,41 @@ def test_a_pair_of_late_inputs_is_replaced_together(calisto): assert "cp_eccentricity_x" not in generated assert generated["cp_eccentricity_y"] is not None assert stochastic.cp_eccentricity_y[0] == 0.6 + + +def test_one_generated_object_cannot_change_the_next_one(calisto_free_form_fins): + """``create_object`` can be called again without a reseed in between. + + The list branch handed back the candidate itself, which for an empty spec is + the model's own working outline, and ``FreeFormFins`` keeps it by reference. + Writing through the first fins therefore reached the second ones. + """ + expected = [tuple(point) for point in calisto_free_form_fins.shape_points] + stochastic = StochasticFreeFormFins( + free_form_fins=calisto_free_form_fins, shape_points=None + ) + + first = stochastic.create_object() + first.shape_points[1] = (9.9, 9.9) + second = stochastic.create_object() + + assert [tuple(point) for point in second.shape_points] == expected + + +def test_the_record_of_a_draw_is_not_a_window_onto_the_object( + calisto_free_form_fins, +): + """``last_rnd_dict`` says what was drawn. + + A Monte Carlo writes it out after the flight has run, so a value the flight + edited in place would be logged instead of the one that was sampled. + """ + stochastic = StochasticFreeFormFins( + free_form_fins=calisto_free_form_fins, shape_points=None + ) + + generated = stochastic.create_object() + recorded = np.array(stochastic.last_rnd_dict["shape_points"], copy=True) + generated.shape_points[1] = (9.9, 9.9) + + assert np.array_equal(stochastic.last_rnd_dict["shape_points"], recorded) From 7c4e8e71190da3a3b1fc865a51d61d3a97ca8c79 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sun, 16 Aug 2026 10:10:13 +0800 Subject: [PATCH 10/16] BUG: record a draw after the subclass that adjusts it, not before Recording in the base generator put the record before StochasticFreeFormFins had pulled the fin root back onto the body line. Under seed 7 the two root points drifted to 0.000299 and 0.001340, the correction returned them to zero, and the record kept the outline the fins were never built from. The rocket copies each component's record into its own, so the Monte Carlo input log carried it too. That is the failure class #1090 was about, arriving from the other side. _record_draw is the one place a model publishes what it drew, and a subclass that changes a value calls it again. A source scan holds the next subclass to the same rule, since the one that gets it wrong is the one nobody wrote a fixture for. _declare_stochastic_input and the _MISSING sentinel had no callers left after the grouped reconfiguration landed, and the first still carried the None handling that #1171 was about, so they are gone rather than left as a second lifecycle for someone to reach for. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- .../stochastic/stochastic_aero_surfaces.py | 3 + rocketpy/stochastic/stochastic_model.py | 33 +++---- rocketpy/stochastic/stochastic_rocket.py | 4 +- .../unit/stochastic/test_stochastic_model.py | 97 ++++++++++++++++++- 4 files changed, 112 insertions(+), 25 deletions(-) diff --git a/rocketpy/stochastic/stochastic_aero_surfaces.py b/rocketpy/stochastic/stochastic_aero_surfaces.py index 137a00385..97017b3bf 100644 --- a/rocketpy/stochastic/stochastic_aero_surfaces.py +++ b/rocketpy/stochastic/stochastic_aero_surfaces.py @@ -623,6 +623,9 @@ def dict_generator(self): generated_dict["shape_points"] = self._keep_root_on_body_line( self.shape_points[0], generated_dict["shape_points"] ) + # The outline moved after the base class recorded it, and what the + # fins are built from is what the record has to hold. + self._record_draw(generated_dict) yield generated_dict @staticmethod diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index 23855d982..e8a6d5ec0 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -83,9 +83,6 @@ def _sampler_seed(seed, input_names): return sum(int(word) << (32 * position) for position, word in enumerate(words)) -_MISSING = object() - - # TODO: Stop using assert in production code. Use exceptions instead. # TODO: Each validation method should have a test case. @@ -151,6 +148,16 @@ def __init__(self, obj, seed=None, **kwargs): self.__nominal_values = {} self._set_stochastic(seed) + def _record_draw(self, generated_dict): + """Records what this model published, after any subclass has finished. + + A record of the draw rather than a window onto what was built from it, + since the object handed those values can be written through afterwards. + A subclass that adjusts a value has to call this again, or the record + keeps what it replaced. + """ + self.last_rnd_dict = _snapshot_of(generated_dict) + def _nominal(self, input_name, getter=getattr): """Returns what ``self.obj`` held for ``input_name`` when it was configured. @@ -197,22 +204,6 @@ def _reconfigure_stochastic_inputs(self, inputs, validate): self.__stochastic_dict[name] = value return validated - def _declare_stochastic_input(self, input_name, input_value): - """Declare an input that an ``add_*`` method installs after ``__init__``. - - ``dict_generator`` walks the inputs a model declared rather than every - attribute on it (#1109), and that list is built in ``__init__``. Anything - added afterwards is set on the instance and never drawn from unless it - says so here. - - The value is the argument as given, not the validated form, because - ``_set_stochastic`` validates it again on every reseed and binds the - distribution to the generator that is live then. - """ - if input_value is None: - return - self.__stochastic_dict[input_name] = input_value - def _set_stochastic(self, seed=None): """Set the stochastic attributes from the input dictionary. This method is useful to reset or reseed the attributes of the instance. @@ -808,9 +799,7 @@ def dict_generator(self): raise RuntimeError( f"An error occurred in the 'sample' method of {arg} CustomSampler" ) from e - # A record of what was drawn, not a window onto what was built from - # it: the object handed the values can be written through afterwards. - self.last_rnd_dict = _snapshot_of(generated_dict) + self._record_draw(generated_dict) yield generated_dict # pylint: disable=too-many-statements diff --git a/rocketpy/stochastic/stochastic_rocket.py b/rocketpy/stochastic/stochastic_rocket.py index 51e80a1cb..438ce6b52 100644 --- a/rocketpy/stochastic/stochastic_rocket.py +++ b/rocketpy/stochastic/stochastic_rocket.py @@ -31,7 +31,7 @@ StochasticTail, StochasticTrapezoidalFins, ) -from .stochastic_model import StochasticModel, _snapshot_of +from .stochastic_model import StochasticModel from .stochastic_parachute import StochasticParachute from .stochastic_solid_motor import StochasticSolidMotor @@ -682,7 +682,7 @@ def dict_generator(self): generated_dict["rail_buttons"] = [] generated_dict["air_brakes"] = [] generated_dict["parachutes"] = [] - self.last_rnd_dict = _snapshot_of(generated_dict) + self._record_draw(generated_dict) yield generated_dict def _create_motor(self, component_stochastic_motor): diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index 18c2ed296..3504c4e91 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -1,6 +1,11 @@ +import ast +import inspect +import textwrap + import numpy as np import pytest +import rocketpy.stochastic as stochastic_package from rocketpy import Environment from rocketpy.mathutils.function import Function from rocketpy.rocket.aero_surface import FreeFormFins @@ -9,7 +14,7 @@ StochasticFreeFormFins, StochasticRocket, ) -from rocketpy.stochastic.stochastic_model import _snapshot_of +from rocketpy.stochastic.stochastic_model import StochasticModel, _snapshot_of @pytest.mark.parametrize( @@ -467,3 +472,93 @@ def test_the_record_of_a_draw_is_not_a_window_onto_the_object( generated.shape_points[1] = (9.9, 9.9) assert np.array_equal(stochastic.last_rnd_dict["shape_points"], recorded) + + +def test_a_subclass_that_adjusts_a_draw_records_it_again(): + """The base class records before a subclass has had its turn. + + ``StochasticFreeFormFins`` pulls the fin root back onto the body line after + ``super().dict_generator()`` has already recorded, so the record held the + outline the correction replaced. Read off the source, because the subclass + that gets this wrong is the one nobody wrote a fixture for. + """ + offenders = [] + for name in dir(stochastic_package): + model = getattr(stochastic_package, name) + if not isinstance(model, type) or not issubclass(model, StochasticModel): + continue + if "dict_generator" not in vars(model): + continue + source = textwrap.dedent(inspect.getsource(model.dict_generator)) + body = ast.parse(source).body[0].body + writes = [ + node + for node in ast.walk(ast.Module(body=body, type_ignores=[])) + if isinstance(node, ast.Assign) + and any(isinstance(target, ast.Subscript) for target in node.targets) + ] + records = "_record_draw" in source + if writes and not records: + offenders.append(name) + + assert not offenders, ( + f"these override dict_generator, write into the drawn dictionary and " + f"never call _record_draw, so the record keeps what they replaced: " + f"{sorted(offenders)}" + ) + + +def test_the_record_holds_the_outline_the_fins_were_built_from( + calisto_free_form_fins, +): + """A perturbed outline is pulled back onto the body line after it is drawn. + + Under seed 7 the two root points drift to 0.000299 and 0.001340 and the + correction returns them to zero, so a record taken before it reports an + outline the fins were never built from. + """ + stochastic = StochasticFreeFormFins( + free_form_fins=calisto_free_form_fins, shape_points=0.001 + ) + stochastic._set_stochastic(7) + + built = stochastic.create_object() + + assert np.array_equal( + np.asarray(stochastic.last_rnd_dict["shape_points"], dtype=float), + np.asarray(built.shape_points, dtype=float), + ) + + +def test_a_rocket_records_the_outline_its_fins_were_built_from( + stochastic_calisto, stochastic_free_form_fins +): + """The same, once the fins are nested in a rocket. + + ``_create_surface`` copies each component's own record into the rocket's, + so a component that recorded too early reaches the Monte Carlo input log. + """ + # A tuple position carries its own centre, so it does not go looking for + # matching fins on a deterministic rocket that has none. + stochastic_calisto.add_free_form_fins( + stochastic_free_form_fins, position=(-1.05, 0.001) + ) + stochastic_calisto._set_stochastic(7) + + rocket = stochastic_calisto.create_object() + + recorded = next( + entry["shape_points"] + for entry in stochastic_calisto.last_rnd_dict["aerodynamic_surfaces"] + if "shape_points" in entry + ) + built = next( + surface + for surface in rocket.aerodynamic_surfaces.get_components() + if isinstance(surface, FreeFormFins) + ) + + assert np.array_equal( + np.asarray(recorded, dtype=float), + np.asarray(built.shape_points, dtype=float), + ) From d069cb9c5b4eb06583edd8d189904363e6c22250 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Mon, 17 Aug 2026 02:44:17 +0800 Subject: [PATCH 11/16] DOC: _choose returns a copy, and say so where it is documented The docstring still promised values itself when there are no candidates, which stopped being true when the draw started handing back a copy. Nothing reaches that branch through a validated input, since an empty list validates to the object's own value, so it is a guard against integers(0) rather than a path with a caller. It has a test now, which is also the one line of this change Codecov had no coverage for. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/stochastic/stochastic_model.py | 6 +++--- tests/unit/stochastic/test_stochastic_model.py | 12 ++++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index e8a6d5ec0..1bc1caf68 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -273,12 +273,12 @@ def _choose(self, values): Returns ------- object - One of the candidates, or ``values`` itself when there are none. + A copy of one of the candidates, or of ``values`` when there are + none. Copied because what this returns is handed to the object + being built. """ if len(values) == 0: return _snapshot_of(values) - # Copied, because what this returns is handed to the object being built - # and would otherwise be the candidate the next draw picks again. return _snapshot_of(values[self.__choice_generator.integers(len(values))]) def _nominal_value(self, input_name, value): diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index 3504c4e91..d4c440d6c 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -562,3 +562,15 @@ def test_a_rocket_records_the_outline_its_fins_were_built_from( np.asarray(recorded, dtype=float), np.asarray(built.shape_points, dtype=float), ) + + +def test_choosing_between_no_candidates_gives_back_an_empty_copy(example_plain_env): + """Validation never produces an empty candidate list, so this is the guard + that keeps ``integers(0)`` from raising if one ever reaches here.""" + model = StochasticEnvironment(environment=example_plain_env) + empty = [] + + chosen = model._choose(empty) + + assert chosen == [] + assert chosen is not empty From 16a73b4b31bee79734559a948d8eead9601223e2 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Mon, 17 Aug 2026 02:59:08 +0800 Subject: [PATCH 12/16] BUG: record the parachute noise seed the parachute was built with StochasticParachute.create_object derives the pressure noise seed after the draw, and #1134 relied on last_rnd_dict being the same dictionary to carry it into the record. Snapshotting the draw broke that link: the parachute is still built with the seed, but the record loses it, so the Monte Carlo inputs stop describing the parachute that flew. develop recorded 37773913418288439290323614982376424810 before recorded The source scan missed it because it only read dict_generator overrides. It reads create_object too now, and tracks the names a method binds from a draw rather than guessing at a variable name, so a local a method fills in for its own use is not mistaken for a record. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/stochastic/stochastic_parachute.py | 3 + .../unit/stochastic/test_stochastic_model.py | 82 +++++++++++++++---- 2 files changed, 69 insertions(+), 16 deletions(-) diff --git a/rocketpy/stochastic/stochastic_parachute.py b/rocketpy/stochastic/stochastic_parachute.py index c1b24e365..6fe73dd69 100644 --- a/rocketpy/stochastic/stochastic_parachute.py +++ b/rocketpy/stochastic/stochastic_parachute.py @@ -211,4 +211,7 @@ def create_object(self): generated_dict["seed"] = _sampler_seed( self._seed, ("pressure_noise", generated_dict["name"]) ) + # Recorded after the seed is in, or the inputs describe a parachute + # with noise nobody can reproduce. + self._record_draw(generated_dict) return Parachute(**generated_dict) diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index d4c440d6c..2aab14e63 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -12,6 +12,7 @@ from rocketpy.stochastic import ( StochasticEnvironment, StochasticFreeFormFins, + StochasticParachute, StochasticRocket, ) from rocketpy.stochastic.stochastic_model import StochasticModel, _snapshot_of @@ -477,9 +478,9 @@ def test_the_record_of_a_draw_is_not_a_window_onto_the_object( def test_a_subclass_that_adjusts_a_draw_records_it_again(): """The base class records before a subclass has had its turn. - ``StochasticFreeFormFins`` pulls the fin root back onto the body line after - ``super().dict_generator()`` has already recorded, so the record held the - outline the correction replaced. Read off the source, because the subclass + ``StochasticFreeFormFins`` corrects the outline in ``dict_generator`` and + ``StochasticParachute`` adds the pressure noise seed in ``create_object``, + both after the record was taken. Read off the source, because the subclass that gets this wrong is the one nobody wrote a fixture for. """ offenders = [] @@ -487,19 +488,38 @@ def test_a_subclass_that_adjusts_a_draw_records_it_again(): model = getattr(stochastic_package, name) if not isinstance(model, type) or not issubclass(model, StochasticModel): continue - if "dict_generator" not in vars(model): - continue - source = textwrap.dedent(inspect.getsource(model.dict_generator)) - body = ast.parse(source).body[0].body - writes = [ - node - for node in ast.walk(ast.Module(body=body, type_ignores=[])) - if isinstance(node, ast.Assign) - and any(isinstance(target, ast.Subscript) for target in node.targets) - ] - records = "_record_draw" in source - if writes and not records: - offenders.append(name) + for method in ("dict_generator", "create_object"): + if method not in vars(model): + continue + source = textwrap.dedent(inspect.getsource(vars(model)[method])) + body = ast.parse(source).body[0].body + # Only the names the method binds from a draw. A local it fills + # in for its own use, such as the factors StochasticEnvironment + # collects, is not a record of anything. + drawn = { + target.id + for node in ast.walk(ast.Module(body=body, type_ignores=[])) + if isinstance(node, ast.Assign) + and ( + "dict_generator" in ast.dump(node.value) + or method == "dict_generator" + ) + for target in node.targets + if isinstance(target, ast.Name) + } + writes = [ + node + for node in ast.walk(ast.Module(body=body, type_ignores=[])) + if isinstance(node, ast.Assign) + and any( + isinstance(target, ast.Subscript) + and isinstance(target.value, ast.Name) + and target.value.id in drawn + for target in node.targets + ) + ] + if writes and "_record_draw" not in source: + offenders.append(f"{name}.{method}") assert not offenders, ( f"these override dict_generator, write into the drawn dictionary and " @@ -574,3 +594,33 @@ def test_choosing_between_no_candidates_gives_back_an_empty_copy(example_plain_e assert chosen == [] assert chosen is not empty + + +def test_a_parachute_records_the_noise_seed_it_was_built_with(calisto_main_chute): + """``create_object`` derives the pressure noise seed after the draw. + + The parachute is built with it either way, so a record taken before it + describes a parachute whose noise nobody can reproduce. + """ + stochastic = StochasticParachute(parachute=calisto_main_chute, cd_s=0.1, lag=0.2) + stochastic._set_stochastic(42) + + built = stochastic.create_object() + + assert stochastic.last_rnd_dict["seed"] == built._seed + + +def test_a_rocket_records_the_noise_seed_its_parachute_was_built_with( + stochastic_calisto, calisto_main_chute +): + """The same once nested, which is the shape a Monte Carlo writes out.""" + stochastic_calisto.parachutes = [] + stochastic_calisto.add_parachute( + StochasticParachute(parachute=calisto_main_chute, cd_s=0.1, lag=0.2) + ) + stochastic_calisto._set_stochastic(42) + + rocket = stochastic_calisto.create_object() + + recorded = stochastic_calisto.last_rnd_dict["parachutes"][0]["seed"] + assert recorded == rocket.parachutes[0]._seed From 70b47cac778d4a049968346507ce27bb0d2ca6d0 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Mon, 17 Aug 2026 03:14:08 +0800 Subject: [PATCH 13/16] TST: walk the subclass tree, not the package exports StochasticMotorModel is a StochasticModel subclass that rocketpy.stochastic does not export, so the scan could not see it. It overrides neither method today, which is why nothing was wrong, and which is also why the gap would have gone unnoticed until something did. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- tests/unit/stochastic/test_stochastic_model.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index 2aab14e63..fd1a67322 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -5,7 +5,6 @@ import numpy as np import pytest -import rocketpy.stochastic as stochastic_package from rocketpy import Environment from rocketpy.mathutils.function import Function from rocketpy.rocket.aero_surface import FreeFormFins @@ -483,11 +482,17 @@ def test_a_subclass_that_adjusts_a_draw_records_it_again(): both after the record was taken. Read off the source, because the subclass that gets this wrong is the one nobody wrote a fixture for. """ + # Walked from the base class rather than from what the package exports, + # since StochasticMotorModel is a subclass the exports do not reach. + models, stack = set(), [StochasticModel] + while stack: + for subclass in stack.pop().__subclasses__(): + models.add(subclass) + stack.append(subclass) + offenders = [] - for name in dir(stochastic_package): - model = getattr(stochastic_package, name) - if not isinstance(model, type) or not issubclass(model, StochasticModel): - continue + for model in sorted(models, key=lambda cls: cls.__name__): + name = model.__name__ for method in ("dict_generator", "create_object"): if method not in vars(model): continue From 43c5bd11962a41dd9e0d07623c7fbc2ebe8b8de1 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Mon, 17 Aug 2026 03:24:57 +0800 Subject: [PATCH 14/16] BUG: stop an omitted axis from taking away what was declared Removing on None looked like one line inside the new replacement helper, and it is not. add_cp_eccentricity(x=..., y=...) defaults both to None, so an omitted axis and an explicit None read identically, and the removal took away an axis the caller never mentioned: add_cp_eccentricity(x=0.001, y=0.002) add_cp_eccentricity(x=0.005) # y quietly gone develop keeps y here, and so does this again. Removing an earlier declaration needs an argument omission cannot supply, which is a signature change and its own decision, so it stays in #1171 rather than arriving inside a change about nominal ownership. The test that asked for removal is replaced by one that holds the omitted axis in place, since that is the behaviour anything already written depends on. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/stochastic/stochastic_model.py | 10 +++++---- .../unit/stochastic/test_stochastic_model.py | 21 ++++++++++--------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index 1bc1caf68..11661159e 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -182,7 +182,11 @@ def _reconfigure_stochastic_inputs(self, inputs, validate): has to drop it before validation reads one. Whatever a caller passes together is replaced together, since ``add_cp_eccentricity`` takes x and y in one call and a y that will not validate must not leave x - already replaced. ``None`` takes away what was declared before. + already replaced. + + ``None`` leaves any earlier declaration alone. It reads the same way + whether the caller wrote it or left the argument out, and removing on + the second reading would take away an axis nobody mentioned. """ inputs = tuple(inputs) kept = { @@ -198,9 +202,7 @@ def _reconfigure_stochastic_inputs(self, inputs, validate): self.__nominal_values.update(kept) raise for name, value in inputs: - if value is None: - self.__stochastic_dict.pop(name, None) - else: + if value is not None: self.__stochastic_dict[name] = value return validated diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index fd1a67322..26216cb86 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -379,22 +379,23 @@ def test_a_refused_reconfiguration_leaves_the_previous_one(calisto): assert stochastic.cp_eccentricity_x[0] == 0.5 -def test_taking_a_late_input_away_removes_what_it_declared(calisto): - """``None`` is a configuration too, and it has to survive a reseed. +def test_leaving_an_axis_out_does_not_take_away_what_it_declared(calisto): + """``None`` reads the same whether it was written or the argument was + left out, so a call that mentions only x has to leave y where it was. - The nominal was refreshed but the earlier distribution stayed declared, so - the next ``_set_stochastic`` validated it again and drew an uncertainty the - caller had asked to remove. + Removing on the second reading would take away an axis nobody mentioned, + which is why removal needs an argument the caller cannot supply by + omission. That is #1171 rather than this change. """ stochastic = StochasticRocket(rocket=calisto, radius=0.0127 / 2) - stochastic.add_cp_eccentricity(x=0.001) + stochastic.add_cp_eccentricity(x=0.001, y=0.002) - calisto.cp_eccentricity_x = 0.8 - stochastic.add_cp_eccentricity(x=None) + stochastic.add_cp_eccentricity(x=0.005) stochastic._set_stochastic(11) - assert "cp_eccentricity_x" not in next(stochastic.dict_generator()) - assert stochastic.cp_eccentricity_x == [0.8] + generated = next(stochastic.dict_generator()) + assert "cp_eccentricity_x" in generated + assert "cp_eccentricity_y" in generated def test_a_half_that_was_never_given_is_not_declared(calisto): From 6779dbc06ab57fff93d5a744050f554f4fe162bd Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Mon, 17 Aug 2026 12:59:25 +0800 Subject: [PATCH 15/16] DOC: say what a second add_cp_eccentricity call does Both arguments read as optional and nothing said what happens when the method is called again, which is the whole of the question behind #1171. Each public docstring now states it: a later call replaces what was configured, an omitted axis keeps what it had, and taking one away is not supported. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/stochastic/stochastic_rocket.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/rocketpy/stochastic/stochastic_rocket.py b/rocketpy/stochastic/stochastic_rocket.py index 438ce6b52..915d247c9 100644 --- a/rocketpy/stochastic/stochastic_rocket.py +++ b/rocketpy/stochastic/stochastic_rocket.py @@ -451,6 +451,12 @@ def add_cp_eccentricity(self, x=None, y=None): the y direction relative to the center of dry mass axial line. The y axis is defined according to the body axes coordinate system. + + Calling this again replaces what was configured before. An axis left + out keeps the setting it already had, since ``None`` is what an omitted + argument arrives as and cannot be told apart from one written by hand. + Taking an axis away again is not supported (#1171). + Returns ------- self : StochasticRocket @@ -481,6 +487,12 @@ def add_thrust_eccentricity(self, x=None, y=None): relative to the center of dry mass axial line. The y axis is defined according to the body axes coordinate system. + + Calling this again replaces what was configured before. An axis left + out keeps the setting it already had, since ``None`` is what an omitted + argument arrives as and cannot be told apart from one written by hand. + Taking an axis away again is not supported (#1171). + Returns ------- self : StochasticRocket From 302fc8bd3f7a1ac95e665df66db8e047766c964c Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:49:12 +0800 Subject: [PATCH 16/16] BUG: leave an axis that was left out entirely alone Keeping its declaration was not enough. The omitted axis still went through the whole replacement: its kept nominal was dropped, None was validated again into a lone nominal, and that was written back over its distribution. The private side then said the axis was random while the attribute dict_generator reads said it was not, so it stopped varying: add_cp_eccentricity(x=0.001, y=0.002) add_cp_eccentricity(x=0.005) eight draws of y -> one distinct value A serial Monte Carlo never resets, so a whole study would have run with that axis switched off and nothing raised. Dropping the nominal also moved the centre. With the rocket's own y changed between the two calls, the next reset centred the old distribution on 9.0 rather than the 0.0 it was configured around. An axis given as None that already has a configuration is now left out of the transaction: not revalidated, its nominal not re-read, its attribute not rewritten. The test covers both eccentricity methods and looks before the reset as well as after, which is where the previous one missed it. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/stochastic/stochastic_model.py | 18 ++++++++-- .../unit/stochastic/test_stochastic_model.py | 35 +++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/rocketpy/stochastic/stochastic_model.py b/rocketpy/stochastic/stochastic_model.py index 11661159e..5771152eb 100644 --- a/rocketpy/stochastic/stochastic_model.py +++ b/rocketpy/stochastic/stochastic_model.py @@ -189,16 +189,28 @@ def _reconfigure_stochastic_inputs(self, inputs, validate): the second reading would take away an axis nobody mentioned. """ inputs = tuple(inputs) + # A None that already has a configuration is an axis the caller left + # out, since an omitted argument arrives the same way. It keeps what it + # had: not validated again, and its kept nominal not read again. + untouched = { + name + for name, value in inputs + if value is None and name in self.__stochastic_dict + } kept = { name: self.__nominal_values.pop(name) for name, _ in inputs - if name in self.__nominal_values + if name not in untouched and name in self.__nominal_values } try: - validated = [validate(name, value) for name, value in inputs] + validated = [ + getattr(self, name) if name in untouched else validate(name, value) + for name, value in inputs + ] except BaseException: for name, _ in inputs: - self.__nominal_values.pop(name, None) + if name not in untouched: + self.__nominal_values.pop(name, None) self.__nominal_values.update(kept) raise for name, value in inputs: diff --git a/tests/unit/stochastic/test_stochastic_model.py b/tests/unit/stochastic/test_stochastic_model.py index 26216cb86..96bf04b3c 100644 --- a/tests/unit/stochastic/test_stochastic_model.py +++ b/tests/unit/stochastic/test_stochastic_model.py @@ -379,6 +379,41 @@ def test_a_refused_reconfiguration_leaves_the_previous_one(calisto): assert stochastic.cp_eccentricity_x[0] == 0.5 +@pytest.mark.parametrize( + ("add_them", "kept"), + [ + ("add_cp_eccentricity", "cp_eccentricity_y"), + ("add_thrust_eccentricity", "thrust_eccentricity_y"), + ], +) +def test_an_axis_left_out_keeps_everything_it_had(calisto, add_them, kept): + """Not only its declaration: its distribution and its kept nominal too. + + Keeping the declaration alone left the two disagreeing. The private side + still said the axis was random while the validated attribute had been + replaced by a lone nominal, and ``dict_generator`` reads the attribute, so + the axis stopped varying until something reset the model. A serial Monte + Carlo never does, so a whole study would have run with it switched off. + """ + setattr(calisto, kept, 0.0) + stochastic = StochasticRocket(rocket=calisto, radius=0.0127 / 2) + getattr(stochastic, add_them)(x=0.001, y=0.002) + stochastic._set_stochastic(11) + centre = getattr(stochastic, kept)[0] + + # Moved between the calls, so re-reading the nominal would show up. + setattr(calisto, kept, 9.0) + getattr(stochastic, add_them)(x=0.005) + + drawn = {next(stochastic.dict_generator())[kept] for _ in range(8)} + assert len(drawn) == 8, "the axis stopped varying before any reset" + + stochastic._set_stochastic(11) + + assert getattr(stochastic, kept)[0] == centre + assert kept in next(stochastic.dict_generator()) + + def test_leaving_an_axis_out_does_not_take_away_what_it_declared(calisto): """``None`` reads the same whether it was written or the argument was left out, so a call that mentions only x has to leave y where it was.