Skip to content

Commit 4263fa9

Browse files
thatrandomasiandevGui-FernandesBRclaude
authored
BUG: sample StochasticFlight inputs once per simulation (#1090) (#1126)
* BUG: sample StochasticFlight inputs once per simulation (#1090) * MNT: satisfy ruff format on MonteCarlo.__sim_producer (#1090) --------- Co-authored-by: Gui-FernandesBR <guilherme_fernandes@usp.br> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 1b87124 commit 4263fa9

4 files changed

Lines changed: 74 additions & 19 deletions

File tree

‎rocketpy/simulation/monte_carlo.py‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -588,12 +588,15 @@ def __run_single_simulation(self):
588588
Flight
589589
The flight object of the simulation.
590590
"""
591+
rocket = self.rocket.create_object()
592+
environment = self.environment.create_object()
593+
flight_inputs = self.flight._sample_flight_inputs()
591594
return Flight(
592-
rocket=self.rocket.create_object(),
593-
environment=self.environment.create_object(),
594-
rail_length=self.flight._randomize_rail_length(),
595-
inclination=self.flight._randomize_inclination(),
596-
heading=self.flight._randomize_heading(),
595+
rocket=rocket,
596+
environment=environment,
597+
rail_length=flight_inputs["rail_length"],
598+
inclination=flight_inputs["inclination"],
599+
heading=flight_inputs["heading"],
597600
initial_solution=self.flight.initial_solution,
598601
terminate_on_apogee=self.flight.terminate_on_apogee,
599602
time_overshoot=self.flight.time_overshoot,
@@ -1522,7 +1525,7 @@ def export_ellipses_to_kml( # pylint: disable=too-many-statements
15221525
except KeyError as e:
15231526
raise KeyError("No impact data found. Skipping impact ellipses.") from e
15241527

1525-
(apogee_ellipses, impact_ellipses) = generate_monte_carlo_ellipses(
1528+
apogee_ellipses, impact_ellipses = generate_monte_carlo_ellipses(
15261529
impact_x,
15271530
impact_y,
15281531
apogee_x,

‎rocketpy/stochastic/stochastic_flight.py‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,21 +123,28 @@ def _validate_initial_solution(self, initial_solution):
123123
else:
124124
raise TypeError("`initial_solution` must be a tuple of numbers")
125125

126-
# TODO: these methods call dict_generator a lot of times unnecessarily
126+
def _sample_flight_inputs(self):
127+
"""Sample rail_length, inclination, and heading in a single draw.
128+
129+
Returns
130+
-------
131+
dict
132+
Mapping with keys ``rail_length``, ``inclination``, and ``heading``.
133+
Also updates ``last_rnd_dict``.
134+
"""
135+
return next(self.dict_generator())
136+
127137
def _randomize_rail_length(self):
128138
"""Randomizes the rail length of the flight."""
129-
generated_dict = next(self.dict_generator())
130-
return generated_dict["rail_length"]
139+
return self._sample_flight_inputs()["rail_length"]
131140

132141
def _randomize_inclination(self):
133142
"""Randomizes the inclination of the flight."""
134-
generated_dict = next(self.dict_generator())
135-
return generated_dict["inclination"]
143+
return self._sample_flight_inputs()["inclination"]
136144

137145
def _randomize_heading(self):
138146
"""Randomizes the heading of the flight."""
139-
generated_dict = next(self.dict_generator())
140-
return generated_dict["heading"]
147+
return self._sample_flight_inputs()["heading"]
141148

142149
def create_object(self):
143150
"""Creates and returns a Flight object from the randomly generated input
@@ -148,12 +155,11 @@ def create_object(self):
148155
flight : Flight
149156
Flight object with the randomly generated input arguments.
150157
"""
151-
generated_dict = next(self.dict_generator())
152-
# TODO: maybe we should use generated_dict["rail_length"] instead
158+
generated_dict = self._sample_flight_inputs()
153159
return Flight(
154160
rocket=self.obj.rocket,
155161
environment=self.obj.env,
156-
rail_length=self._randomize_rail_length(),
162+
rail_length=generated_dict["rail_length"],
157163
inclination=generated_dict["inclination"],
158164
heading=generated_dict["heading"],
159165
initial_solution=self.initial_solution,

‎tests/unit/simulation/test_monte_carlo.py‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -614,9 +614,13 @@ def test_a_monte_carlo_flight_keeps_the_configuration_it_was_given(monkeypatch):
614614
initial_solution=None,
615615
terminate_on_apogee=True,
616616
time_overshoot=False,
617-
_randomize_rail_length=lambda: 5.0,
618-
_randomize_inclination=lambda: 84.0,
619-
_randomize_heading=lambda: 133.0,
617+
# One draw for all three, as #1090 requires: three separate calls
618+
# meant the flight flew one sample and the exported row logged another.
619+
_sample_flight_inputs=lambda: {
620+
"rail_length": 5.0,
621+
"inclination": 84.0,
622+
"heading": 133.0,
623+
},
620624
)
621625
analysis = object.__new__(MonteCarlo)
622626
analysis.flight = stochastic_flight
@@ -633,6 +637,12 @@ def test_a_monte_carlo_flight_keeps_the_configuration_it_was_given(monkeypatch):
633637
assert flight.equations_of_motion == "solid_propulsion"
634638
assert flight.simulation_mode == "native"
635639
assert flight.name == "named"
640+
# and the single draw is what the flight is actually built from
641+
assert (flight.rail_length, flight.inclination, flight.heading) == (
642+
5.0,
643+
84.0,
644+
133.0,
645+
)
636646

637647

638648
@pytest.mark.parametrize(

‎tests/unit/stochastic/test_stochastic_flight.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,39 @@ def test_dict_generator_skips_initial_solution_list(flight_calisto_robust):
7171
generated = next(stochastic_flight.dict_generator())
7272
assert "initial_solution" not in generated
7373
assert stochastic_flight.initial_solution == initial_solution
74+
75+
76+
def test_create_object_matches_last_rnd_dict(flight_calisto_robust):
77+
"""Regression for #1090: create_object must use one dict_generator draw.
78+
79+
Spreads are set on all three flight inputs so a second draw would diverge
80+
from ``last_rnd_dict``.
81+
"""
82+
stochastic_flight = StochasticFlight(
83+
flight=flight_calisto_robust,
84+
rail_length=(5.2, 0.5),
85+
inclination=(84.7, 1),
86+
heading=(53, 2),
87+
)
88+
stochastic_flight._set_stochastic(4242)
89+
90+
flight = stochastic_flight.create_object()
91+
sampled = stochastic_flight.last_rnd_dict
92+
93+
assert flight.rail_length == sampled["rail_length"]
94+
assert flight.inclination == sampled["inclination"]
95+
assert flight.heading == sampled["heading"]
96+
97+
98+
def test_monte_carlo_single_simulation_matches_flight_last_rnd_dict(
99+
monte_carlo_calisto,
100+
):
101+
"""Regression for #1090: MonteCarlo must fly the same sample it logs."""
102+
monte_carlo_calisto.flight._set_stochastic(4242)
103+
104+
flight = monte_carlo_calisto._MonteCarlo__run_single_simulation()
105+
sampled = monte_carlo_calisto.flight.last_rnd_dict
106+
107+
assert flight.rail_length == sampled["rail_length"]
108+
assert flight.inclination == sampled["inclination"]
109+
assert flight.heading == sampled["heading"]

0 commit comments

Comments
 (0)