Sampled parachute geometry never reaches the rocket StochasticRocket builds

[thc1006]

Describe the bug

StochasticParachute.create_object() builds a Parachute from a complete draw:

generated_dict = next(self.dict_generator())
return Parachute(**generated_dict)

StochasticRocket.create_object() then throws that object away and builds a second one from six of its ten fields:

parachute = self._create_parachute(parachute)
rocket.add_parachute(
    name=parachute.name,
    cd_s=parachute.cd_s,
    trigger=parachute.trigger,
    sampling_rate=parachute.sampling_rate,
    lag=parachute.lag,
    noise=parachute.noise,
)

radius, height, porosity and drag_coefficient are not passed. Parachute.__init__ accepts all four and Rocket.add_parachute forwards all four, so the omission is silent: the second parachute re-derives radius from cd_s and the default drag coefficient, and height falls back to that radius.

A study that randomizes canopy geometry therefore flies the nominal geometry, while last_rnd_dict records the sampled values it did not use. The inputs log and the simulation disagree, and nothing reports it.

To Reproduce

stochastic_rocket.add_parachute(
    StochasticParachute(
        chute, radius=(2.0, 0.3), height=(1.5, 0.2),
        porosity=(0.05, 0.01), drag_coefficient=(1.4, 0.2),
    )
)
stochastic_rocket._set_stochastic(42)
built = stochastic_rocket.create_object()
logged = stochastic_rocket.last_rnd_dict["parachutes"][0]

Comparing what was drawn against what reached the rocket:

                 sampled                on rocket
cd_s             10.0                   10.0
radius            2.0914151239263292     1.5078600877302688
height            1.6500902391612915     1.5078600877302688
porosity          0.05940564716391214    0.0432
drag_coefficient  1.1920031787519008     1.4

radius and height come out equal because both were re-derived. sqrt(cd_s / 1.4 / pi) is 1.5078600877303, which is the value in both rows: the derivation used the default drag coefficient, not the sampled one.

Expected behavior

The sampled parachute reaches the rocket. _create_parachute has already built exactly the right object, so attaching it rather than rebuilding it would be enough:

rocket.parachutes.append(self._create_parachute(parachute))

That also stops Parachute.__init__ running twice per parachute per simulation, which matters for #1091, since the second run re-initializes the pressure noise from the global NumPy RNG.

A public Rocket.add_parachute_object() would be tidier than reaching into the list, if that is preferred.

Additional context

Not a regression, and not touched by #1054. Found while reviewing that PR.

Related: #1091 (parachute pressure noise outside the seed tree). This bug is upstream of that one: the noise is initialized twice today, so fixing the seed tree without fixing this leaves the second initialization to undo the first.

Verified on develop at 1691119, NumPy 2.5.1, Python 3.12.13.

Signed-off-by: thc1006 84045975+thc1006@users.noreply.github.com

[thc1006]

Sent as #1098.

One correction to what I wrote here. I said the affected set was studies that randomize the geometry. Measuring it, that is too narrow: dropping the four fields does not need anyone to randomize them. A parachute configured with an explicit radius=2.0 and no randomization at all flew 1.5193, because the value was lost on the way to add_parachute rather than sampled away.

The size of it, since radius and height feed the added mass term rather than only the reports: about 71% on added mass for that parachute.

Measuring also turned up a case I had not considered, where the fix is a trade rather than a straight win. Details are in the PR under "One case I would like your call on". Short version: a parachute with derived geometry used to have its radius re-derived downstream from the sampled cd_s, so the canopy tracked the drag area. Attaching the sampled object stops that, and I would rather have a maintainer decide whether to keep the coupling than pick for you.

[thc1006]

Fixed on develop in d74bf3f (#1098). Leaving this open because the closing keyword only fires on the default branch, so it will shut when develop reaches master.

The derived-geometry question from that PR is still unanswered, and it is the one thing here that could still change: a parachute whose radius was omitted now keeps the radius derived from the nominal cd_s instead of tracking the sampled one. If that turns out to be the wrong call, it is a change to StochasticParachute rather than a revert of this.