Describe the bug
StochasticModel.dict_generator walks the whole instance rather than the inputs the model was given:
for arg, value in self.__dict__.items():
if isinstance(value, tuple):
dist_sampler = value[-1]
generated_dict[arg] = dist_sampler(value[0], value[1])
elif isinstance(value, list):
generated_dict[arg] = self._random_choice(value)
Every tuple on the instance is read as (nominal, spread, sampler) and its last element called. StochasticFlight.initial_solution is a documented tuple of 14 numbers, so the last one gets called as if it were a distribution:
TypeError: 'float' object is not callable
The list form does not raise. It picks one of the fourteen state elements at random and puts that in the generated dictionary, while the Flight that gets built uses the whole list. So a deterministic piece of configuration is sampled, and the sample is thrown away.
To Reproduce
stochastic_flight = StochasticFlight(
flight=flight,
inclination=(84.0, 1.0),
initial_solution=tuple(float(i) for i in range(14)),
)
stochastic_flight._set_stochastic(42)
next(stochastic_flight.dict_generator())
tuple of 14 TypeError: 'float' object is not callable
list of 14 initial_solution -> 8.0
_validate_initial_solution exists and accepts both forms, so this is a valid argument rather than a misuse. The constructor does not call it, which is a smaller thing worth fixing alongside.
Expected behavior
initial_solution is not a stochastic input and should not appear in a draw at all:
assert "initial_solution" not in generated
assert stochastic_flight.initial_solution == initial_solution
The narrow fix is another name in an exception list. The fix that closes the class is to generate only from the names the constructor declared, which are already kept in __stochastic_dict. Fields a subclass installs later, ensemble_member among them, still need reaching, so a hasattr guard rather than a plain iteration.
Additional context
Found while reviewing #1054 and reproduced on develop at 235dc6e, so this predates it. It is worth saying what that PR does and does not change here, because the two look similar.
StochasticRocket builds air_brakes and parachutes before calling the base constructor, so they sit early in __dict__ and their draw moved every later one. #1054 made list choices draw from the model's own generator instead of the standard library's, which turned that from harmless into a shift of the whole model, and it fixes those five collections by name.
initial_solution is set after the base constructor, so it sorts last in __dict__ and shifts nothing. That is why it survives as a crash rather than a silent difference, and why a name-by-name list keeps working right up until someone adds a collection in the wrong place.
Signed-off-by: thc1006 84045975+thc1006@users.noreply.github.com
Describe the bug
StochasticModel.dict_generatorwalks the whole instance rather than the inputs the model was given:Every tuple on the instance is read as
(nominal, spread, sampler)and its last element called.StochasticFlight.initial_solutionis a documented tuple of 14 numbers, so the last one gets called as if it were a distribution:The list form does not raise. It picks one of the fourteen state elements at random and puts that in the generated dictionary, while the
Flightthat gets built uses the whole list. So a deterministic piece of configuration is sampled, and the sample is thrown away.To Reproduce
_validate_initial_solutionexists and accepts both forms, so this is a valid argument rather than a misuse. The constructor does not call it, which is a smaller thing worth fixing alongside.Expected behavior
initial_solutionis not a stochastic input and should not appear in a draw at all:The narrow fix is another name in an exception list. The fix that closes the class is to generate only from the names the constructor declared, which are already kept in
__stochastic_dict. Fields a subclass installs later,ensemble_memberamong them, still need reaching, so ahasattrguard rather than a plain iteration.Additional context
Found while reviewing #1054 and reproduced on
developat235dc6e, so this predates it. It is worth saying what that PR does and does not change here, because the two look similar.StochasticRocketbuildsair_brakesandparachutesbefore calling the base constructor, so they sit early in__dict__and their draw moved every later one. #1054 made list choices draw from the model's own generator instead of the standard library's, which turned that from harmless into a shift of the whole model, and it fixes those five collections by name.initial_solutionis set after the base constructor, so it sorts last in__dict__and shifts nothing. That is why it survives as a crash rather than a silent difference, and why a name-by-name list keeps working right up until someone adds a collection in the wrong place.Signed-off-by: thc1006 84045975+thc1006@users.noreply.github.com