Describe the bug
MonteCarlo.__run_single_simulation builds the Flight like this:
rail_length=self.flight._randomize_rail_length(),
inclination=self.flight._randomize_inclination(),
heading=self.flight._randomize_heading(),
Each of those three helpers calls next(self.dict_generator()), and dict_generator() samples rail length, inclination and heading together and overwrites last_rnd_dict. So a single simulation performs three complete draws: the Flight receives rail length from draw 1, inclination from draw 2 and heading from draw 3, while last_rnd_dict, and therefore the row written to .inputs.txt, holds draw 3.
The recorded inputs are not the inputs the flight used.
stochastic_flight.py line 124 already carries a TODO about the repeated calls, and #269 lists the same refactor. What I want to add here is that this is not only wasted sampling. It makes the log disagree with the run.
To Reproduce
Wrap the module-level Flight symbol so the constructor arguments can be captured, run one simulation, and compare against the row that was written:
captured = {}
real_flight = mc_module.Flight
class Spy:
def __init__(self, **kw):
captured.update(kw)
self._f = real_flight(**kw)
def __getattr__(self, name):
return getattr(self._f, name)
monkeypatch.setattr(mc_module, "Flight", Spy)
montecarlo = MonteCarlo(
filename=..., environment=..., rocket=..., flight=stochastic_flight
)
montecarlo.simulate(number_of_simulations=1, random_seed=4242)
logged = json.loads(open(montecarlo.input_file).readline())
With the shared stochastic_flight fixture:
field flown logged match
rail_length 5.2 5.2 True
inclination 84.45562392083522 85.60113460150586 False
heading 58.30962862926018 58.30962862926018 True
heading agrees because it comes from the same draw that last_rnd_dict kept. rail_length agrees only because this fixture gives it no spread. A stochastic rail length disagrees the same way inclination does.
Expected behavior
One draw per simulation, with the row in .inputs.txt equal to what the Flight was constructed with:
flight_inputs = next(self.flight.dict_generator())
return Flight(
...
rail_length=flight_inputs["rail_length"],
inclination=flight_inputs["inclination"],
heading=flight_inputs["heading"],
...
)
A cleaner version would give StochasticFlight one canonical creation API so MonteCarlo does not hand-copy a subset of the Flight constructor arguments at all.
Additional context
This matters more now that #1054 gives each simulation index its own seed. That change makes .inputs.txt reproducible per index across serial and parallel runs, and the determinism tests compare those rows. While the log does not match the flight, two runs agreeing on the log does not prove they flew the same thing.
A regression test for this should assert on the Flight that was actually built rather than on last_rnd_dict.
Describe the bug
MonteCarlo.__run_single_simulationbuilds theFlightlike this:Each of those three helpers calls
next(self.dict_generator()), anddict_generator()samples rail length, inclination and heading together and overwriteslast_rnd_dict. So a single simulation performs three complete draws: theFlightreceives rail length from draw 1, inclination from draw 2 and heading from draw 3, whilelast_rnd_dict, and therefore the row written to.inputs.txt, holds draw 3.The recorded inputs are not the inputs the flight used.
stochastic_flight.pyline 124 already carries a TODO about the repeated calls, and #269 lists the same refactor. What I want to add here is that this is not only wasted sampling. It makes the log disagree with the run.To Reproduce
Wrap the module-level
Flightsymbol so the constructor arguments can be captured, run one simulation, and compare against the row that was written:With the shared
stochastic_flightfixture:headingagrees because it comes from the same draw thatlast_rnd_dictkept.rail_lengthagrees only because this fixture gives it no spread. A stochastic rail length disagrees the same way inclination does.Expected behavior
One draw per simulation, with the row in
.inputs.txtequal to what theFlightwas constructed with:A cleaner version would give
StochasticFlightone canonical creation API soMonteCarlodoes not hand-copy a subset of theFlightconstructor arguments at all.Additional context
This matters more now that #1054 gives each simulation index its own seed. That change makes
.inputs.txtreproducible per index across serial and parallel runs, and the determinism tests compare those rows. While the log does not match the flight, two runs agreeing on the log does not prove they flew the same thing.A regression test for this should assert on the
Flightthat was actually built rather than onlast_rnd_dict.