Skip to content

MonteCarlo draws the flight dictionary three times, so the logged inputs are not the ones flown #1090

Description

@thc1006

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions