BUG: one stochastic component wrapper can be stored on a rocket twice

[thc1006]

What happens

Nothing stops the same stochastic wrapper from being stored twice on one rocket. Components.add does not check, and parachutes and air_brakes are plain lists that append.

chute = StochasticParachute(parachute=calisto_main_chute, cd_s=0.1, lag=0.2)

stochastic_rocket.add_parachute(chute)
stochastic_rocket.add_parachute(chute)
stochastic_rocket._set_stochastic(99)

Both list entries are the one object, so the second reseed overwrites the first:

parachutes[0] is parachutes[1]:   True
_seed of each entry:              154423647178671419759920218774039641014
                                  154423647178671419759920218774039641014

The rocket ends up with two parachutes that are not independent. They draw one after the other from the same generator, and StochasticParachute.create_object derives the pressure noise seed from _seed and the parachute name, which are now the same for both, so the noise is identical rather than merely correlated.

A positioned component is worse. __components_map is keyed by the wrapper, so adding one at two positions leaves only the second position, and both entries are reset from it.

Why it is worth stating

On develop today every nested component shares one seed anyway, so this is not visible as a separate problem. #1170 gives each component its own child, and the guarantee it states is that two components sample independently. A wrapper stored twice is the case where that does not hold, so the contract needs an owner.

Fix

The clean rule is that one stochastic wrapper belongs to one collection entry, and two components need two wrappers. Wrapping the same deterministic object twice stays supported, since each call builds a separate wrapper.

Rejecting by identity at add time reads well and is cheap. Copying the wrapper instead would be worse: it may hold a Function, a callable, a CustomSampler or a shared generator, and copy semantics for those are not something to decide implicitly.

Tests worth having: the same StochasticParachute twice is refused, the same surface wrapper at two positions is refused, and two separate wrappers around one deterministic component are accepted and seeded independently.

Related

Two neighbouring problems, both separate from this one:

• add_air_brakes appends the air brake but keeps a single air_brake_controller, so several air brakes all use the controller from the last call;
• set_rail_buttons is named as a setter but calls add, so calling it twice consumes draws for a set of buttons that create_object then overwrites.

[thc1006]

One more shape of the same problem, which a per-rocket duplicate check would let through: the wrapper can be shared between two rockets.

shared = StochasticParachute(parachute=chute, cd_s=0.1, lag=0.2)

rocket_a.add_parachute(shared)
rocket_b.add_parachute(shared)

rocket_a._set_stochastic(1)
rocket_b._set_stochastic(2)
rocket_a.create_object()

Each rocket holds it once, so nothing looks wrong from either side, but rocket_b's reseed has already replaced the generator and the _seed that rocket_a is about to sample from.

So the rule has to be about the wrapper having one owner, not about one collection having no repeats. Enforcing that across rockets needs an owner marker or a weak reference, and it has to survive being pickled to a Monte Carlo worker, which is why this is worth its own change rather than a check bolted onto add_*.

The narrower alternative is to document that a stochastic component wrapper belongs to one rocket and one entry, and to reject repeats within a rocket. That closes the case that produces identical parachute pressure noise, and leaves the cross-rocket case as a documented rule rather than an enforced one.