Repository navigation
Commit 06bdb4b
BUG: give each StochasticRocket component its own random stream (#1170)
* BUG: give each StochasticRocket component its own random stream
_set_stochastic handed the same seed to the rocket body and to every surface,
motor, rail button and parachute, so two components built from one spec drew
identical values: a main and a drogue with the same cd_s and lag spec drew the
same cd_s and the same lag, every time, and a study of both was a study of one
counted twice.
Air brakes were worse. They are built and sampled in create_object and were
not in the reseed at all, so their values came from wherever the generator had
been left rather than from the seed: 0.683, then 0.586, then 0.488 for one
seed asked three times.
Each component now takes its own child of a SeedSequence root, spawned in a
fixed order so one seed still reproduces the whole rocket. The collections are
named in one place and checked against create_object's own source, since the
collection no fixture populates is the one that gets missed.
Extracted from #1054.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
* DOC: shorten the comments on the component seeding
Measured against the register the repository uses: inline comments in
flight.py average 5.6 words and none of its docstrings run longer than the
code they describe. The three added here were four to seven lines of prose
where a line would do, and the seed helper carried seven lines of docstring
over two lines of code.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
* BUG: leave the rocket body's stream where it was, and isolate each collection
Moving the body to child zero broke every fixed-seed baseline for mass,
radius and the body inputs, and nothing about the nested-component fix needed
that. The body keeps the seed as given now: stochastic_calisto under seed 42
reads mass=14.906007947 on develop and the same here.
Components were also addressed by one global traversal index, so adding a fin
moved every motor, rail button, parachute and air brake. Each collection has
a root of its own now, spawned from the same seed, so an unrelated component
in one of them leaves the others where they were.
The source scan compares the two sets both ways. A collection left in the
reseed after create_object stops using it still spawns a child and moves
every stream after it, which the subset check let through.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
* TST: count the reseeds, and cover two air brakes on one spec
The source scan reads create_object for a literal loop over self.collection,
so a helper, a local alias or a getattr would hide a collection from it.
Counting what each entry actually receives is the check that survives a
refactor, and it is the only one that fails when an entry is reseeded twice.
The air brakes are a plain list and take a different route through the reseed
than the positioned collections, so two of them on one spec are worth their
own case.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
* TST: stop the isolation test from storing one wrapper twice
stochastic_calisto already holds the stochastic_nose_cone fixture, so adding
it again put the test into the state #1172 describes: one wrapper in two
entries, its position overwritten, and two reseeds landing on the same object.
The assertion looked at a different collection and passed anyway. It adds the
deterministic nose now, so add_nose builds a wrapper of its own.
Nothing pinned the body keeping the seed as given either. Reproducibility and
seed uniqueness both hold with the body on a spawned child, so neither would
have noticed it going back there.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
* DOC: say how a rocket's components are seeded
The change moves every fixed-seed component baseline and nothing in the user
documentation said how components are seeded at all, before or after.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
* DOC: say what separate streams do and do not promise
Two independent streams are not made to consume the same draws; they can
still land on equal values, and a specification with no spread always will.
The text promised unequal results, which is a stronger claim than spawning
gives.
It also said each kind of component is spawned separately. The unit is the
collection: a nose cone, the fins and the tail share one root.
And a stream belongs to one wrapper, so storing one twice or sharing it
between rockets is outside what this establishes. That is #1172.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
* DOC: say that the reset builds the tree, not the add
A rocket resets itself while being constructed, when it holds no components
yet, so a parachute added afterwards keeps the generator it was built with
until the next reset. The text read as though attaching a component gave it a
stream, which is only true once something resets the rocket, and a Monte Carlo
is what does that.
Two wrappers sharing a CustomSampler seed_group are also one stream on
purpose. Separate component streams are not meant to take that apart, so the
note says so rather than leaving it to be discovered.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
* DOC: correct what Monte Carlo and a shared sampler group actually do
A serial MonteCarlo run never resets the rocket, and a parallel one resets
each worker once rather than once per simulation, so the text saying a run
resets the rocket for you was wrong for both. Per-simulation reset is the
Monte Carlo seeding work, not this change.
CustomSampler.seed_group already documents that a group belongs to one model
and that the last to seed it wins. Saying two components sharing one stay one
stream on purpose read as a guarantee this does not make.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
* DOC: the parallel path does not get as far as building the tree
Saying it resets each worker once reads as though it works and only the
grain differs. It hands the model a SeedSequence where an integer is wanted,
so it stops before the tree exists, which the PR already records as the Monte
Carlo seeding work rather than this change.
The two air brake test also says what it is not: both are added with one
controller because the rocket keeps a single one, which is #1172.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
* TST: pin every word of the seed, and say what append-only means
Dropping the fourth word left all 33 tools tests passing: the checks were
that the high bits are not zero, that the low word matches, that two children
differ and that reading twice agrees, none of which a 96 bit truncation
breaks. It compares against the integer rebuilt from all four words now, and
that mutation fails.
A collection's stream is addressed by where its name falls in the two tuples
read end to end, so appending to the first moves every name in the second.
The comment said append rather than reorder, which reads as though appending
to either one is safe.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
* BUG: take the seed type a parallel run hands the rocket
A parallel run spawns a SeedSequence per worker and passes it down, and
SeedSequence does not take another one as entropy, so rooting the
collections from it raised TypeError.
It was unreachable until now: the base _set_stochastic refuses the same
type one frame earlier, so a worker never got this far. Once that is
fixed the call here is the next one to fail, which is why it is fixed
in the same series rather than left for whoever hits it.
Copied from the full state rather than spawned from directly. spawn()
advances the counter of an object the caller still holds, and a second
use of the same seed would then build the components a different tree.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
* MNT: drop the _seed_sequence_to_int this branch no longer needs to add
#1181 landed the same helper on develop, with the same body, so merging
develop in left tools.py defining it twice a hundred lines apart. Git had no
conflict to report: the two copies were added at different points in the
file, so the second simply shadowed the first, and pylint would have failed
the branch with E0102 rather than anything explaining why.
Develop's copy is kept, its docstring being the fuller of the two. What this
branch still adds on its own is _seed_sequence_from, which is left where it
was, and the test in test_tools.py now covers the surviving definition.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Co-authored-by: Gui-FernandesBR <guilherme.fernandes@maggu.ai>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>1 parent fa38028 commit 06bdb4b
5 files changed
Lines changed: 420 additions & 11 deletions
File tree
- docs/user
- rocketpy
- stochastic
- tests/unit
- stochastic
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
278 | 278 | | |
279 | 279 | | |
280 | 280 | | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
281 | 320 | | |
282 | 321 | | |
283 | 322 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
| |||
21 | 22 | | |
22 | 23 | | |
23 | 24 | | |
| 25 | + | |
24 | 26 | | |
25 | 27 | | |
26 | 28 | | |
| |||
173 | 175 | | |
174 | 176 | | |
175 | 177 | | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
176 | 189 | | |
177 | 190 | | |
178 | 191 | | |
179 | 192 | | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
180 | 198 | | |
181 | 199 | | |
182 | 200 | | |
183 | 201 | | |
184 | 202 | | |
185 | 203 | | |
186 | | - | |
187 | | - | |
188 | | - | |
189 | | - | |
190 | | - | |
191 | | - | |
192 | | - | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
193 | 215 | | |
194 | | - | |
| 216 | + | |
195 | 217 | | |
196 | 218 | | |
197 | 219 | | |
| |||
200 | 222 | | |
201 | 223 | | |
202 | 224 | | |
203 | | - | |
204 | | - | |
| 225 | + | |
| 226 | + | |
205 | 227 | | |
206 | 228 | | |
207 | 229 | | |
| |||
213 | 235 | | |
214 | 236 | | |
215 | 237 | | |
216 | | - | |
| 238 | + | |
217 | 239 | | |
218 | 240 | | |
219 | 241 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1479 | 1479 | | |
1480 | 1480 | | |
1481 | 1481 | | |
| 1482 | + | |
| 1483 | + | |
| 1484 | + | |
| 1485 | + | |
| 1486 | + | |
| 1487 | + | |
| 1488 | + | |
| 1489 | + | |
| 1490 | + | |
| 1491 | + | |
| 1492 | + | |
| 1493 | + | |
1482 | 1494 | | |
1483 | 1495 | | |
1484 | 1496 | | |
| |||
0 commit comments