From 52d6bc418349005ca415d3f4a45c21c07dd24719 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Sat, 15 Aug 2026 23:08:48 +0800 Subject: [PATCH] BUG: draw each declared eccentricity once per simulation #1167 declared the eccentricities that add_cp_eccentricity and add_thrust_eccentricity install, so dict_generator draws them. _create_eccentricities then drew them a second time and overwrote the first value. The exported inputs matched the applied ones only because the second write wins, and the extra draw moved every component position create_object places after it. Read what has been drawn already, and draw only a half the caller left out, which is not a declared input and so never reaches dict_generator. The docstring correction is the explanation #1167 landed with, which named the wrong symptom: the value did vary between simulations, what a fixed seed failed to do was reproduce it. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- rocketpy/stochastic/stochastic_rocket.py | 14 ++++-- .../unit/stochastic/test_stochastic_rocket.py | 48 ++++++++++++++++++- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/rocketpy/stochastic/stochastic_rocket.py b/rocketpy/stochastic/stochastic_rocket.py index 515439f14..65cfb5ebe 100644 --- a/rocketpy/stochastic/stochastic_rocket.py +++ b/rocketpy/stochastic/stochastic_rocket.py @@ -734,10 +734,16 @@ def _create_parachute(self, stochastic_parachute): return parachute def _create_eccentricities(self, stochastic_x, stochastic_y, eccentricity): - x_rnd = self._randomize_position(stochastic_x) - self.last_rnd_dict[eccentricity + "_x"] = x_rnd - y_rnd = self._randomize_position(stochastic_y) - self.last_rnd_dict[eccentricity + "_y"] = y_rnd + # A half that was given is a declared input, so dict_generator has drawn + # it already; drawing again would spend a second value out of the same + # stream and move every component position that follows. + def drawn_once(name, stochastic): + if name not in self.last_rnd_dict: + self.last_rnd_dict[name] = self._randomize_position(stochastic) + return self.last_rnd_dict[name] + + x_rnd = drawn_once(eccentricity + "_x", stochastic_x) + y_rnd = drawn_once(eccentricity + "_y", stochastic_y) return x_rnd, y_rnd def create_object(self): diff --git a/tests/unit/stochastic/test_stochastic_rocket.py b/tests/unit/stochastic/test_stochastic_rocket.py index dcf94df36..fd933aec5 100644 --- a/tests/unit/stochastic/test_stochastic_rocket.py +++ b/tests/unit/stochastic/test_stochastic_rocket.py @@ -212,8 +212,8 @@ def test_an_eccentricity_added_after_init_is_still_drawn(calisto, add_them, name """``dict_generator`` walks the declared inputs, and these arrive later. The list is built in ``__init__``, so a distribution installed by an - ``add_*`` method afterwards was set on the instance and never drawn from: - every simulation used the same value, with nothing to say so. + ``add_*`` method afterwards was never re-validated on a reseed and stayed + bound to the unseeded generator: a fixed seed did not reproduce it. """ stochastic = StochasticRocket(rocket=calisto, radius=0.0127 / 2) getattr(stochastic, add_them)(x=(0.0, 0.001), y=(0.0, 0.001)) @@ -236,3 +236,47 @@ def drawn(seed): assert drawn(7) == drawn(7) assert drawn(7) != drawn(8) + + +def test_a_declared_eccentricity_is_not_drawn_a_second_time(calisto): + """``create_object`` applies the draw ``dict_generator`` already made. + + A second draw spends another value out of the same stream, which moves + every component position ``create_object`` places after it. + """ + stochastic = StochasticRocket(rocket=calisto, radius=0.0127 / 2) + stochastic.add_cp_eccentricity(x=(0.0, 0.01), y=(0.0, 0.01)) + stochastic.add_thrust_eccentricity(x=(0.0, 0.01), y=(0.0, 0.01)) + + stochastic._set_stochastic(42) + declared = next(stochastic.dict_generator()) + expected = {name: declared[name] for name in declared if "eccentricity" in name} + assert len(expected) == 4 + + stochastic._set_stochastic(42) + rocket = stochastic.create_object() + + applied = { + "cp_eccentricity_x": rocket.cp_eccentricity_x, + "cp_eccentricity_y": rocket.cp_eccentricity_y, + "thrust_eccentricity_x": rocket.thrust_eccentricity_x, + "thrust_eccentricity_y": rocket.thrust_eccentricity_y, + } + assert applied == expected + assert {name: stochastic.last_rnd_dict[name] for name in expected} == expected + + +def test_an_eccentricity_half_that_was_left_out_is_still_drawn(calisto): + """Only a half that was given is a declared input, so the other is not. + + ``create_object`` has to keep drawing it, and keep reporting it, or the + inputs it writes stop describing the rocket it built. + """ + stochastic = StochasticRocket(rocket=calisto, radius=0.0127 / 2) + stochastic.add_cp_eccentricity(x=(0.0, 0.01)) + + stochastic._set_stochastic(42) + rocket = stochastic.create_object() + + assert "cp_eccentricity_y" in stochastic.last_rnd_dict + assert stochastic.last_rnd_dict["cp_eccentricity_y"] == rocket.cp_eccentricity_y