Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 7 additions & 16 deletions rocketpy/stochastic/custom_sampler.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,18 @@ class CustomSampler(ABC):

@property
def seed_group(self):
"""The generator state this sampler shares, if it shares one.
"""The generator this sampler shares, or ``self`` if it shares none.

Samplers are independent by default and each is seeded on its own. Two
wrappers over one generator, as the correlated wind pair in the
documentation are, should both return that generator here, so the pair
is seeded once as a unit rather than one of them silently overwriting
the other's seed.

Return the same object on every call. Building the answer each time,
which a property invites, gives each member a different identity and
puts it back in a group of its own.

A group belongs to one model. Declaring the same generator on two
models has them both seed it, and whichever is seeded last decides the
stream, which is the overwrite this is here to avoid.
Samplers sharing a generator must all return it, so the group is seeded
once rather than each member overwriting the previous seed. Return the
same object every call: a rebuilt one has a new identity and forms a
group of its own. A group belongs to one model; declared on two models,
each seeds it and the last one wins.

Returns
-------
object
Identity is what counts, not equality. ``self`` by default, which
makes every sampler its own group.
Matched by identity, not equality. Defaults to ``self``.
"""
return self

Expand Down
36 changes: 12 additions & 24 deletions rocketpy/stochastic/stochastic_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -585,39 +585,30 @@ def _validate_positive_int_list(self, input_name, input_value):
)

def _reset_custom_samplers(self, seed):
"""Give each sampler its own stream, and each shared group one between
them.
"""Seed each sampler, and each shared generator once.

Samplers that share a generator, as the documented wind pair do, are
seeded once as a unit. Resetting each member in turn would leave every
seed but the last discarded and the group's stream decided by whichever
member happened to go last.

Its own pass rather than the validation loop below, whose order sets
``__dict__`` and so the order every other input is drawn in.
Kept out of the validation loop in ``_set_stochastic``, whose order
sets ``__dict__`` and with it the order every other input is drawn in.
"""
groups = {}
for input_name in sorted(self.__stochastic_dict):
sampler = self.__stochastic_dict[input_name]
if isinstance(sampler, CustomSampler):
# Held in the value as well as keyed on, because `id` is
# unique only among live objects. Defensive: a `seed_group`
# that builds its answer did not merge in practice here.
# Kept in the value too: `id` is unique only among live
# objects, so the group has to outlive the dict.
group = sampler.seed_group
shared = groups.setdefault(id(group), ([], sampler, group))
shared[0].append(input_name)

for names, sampler, group in groups.values():
# The group itself when it can be reset, since it is the thing that
# holds the shared state. Going through one member instead assumes
# every member resets the same way and keeps nothing of its own.
# The group holds the shared state, so reset it directly; a member
# may reset differently, or keep state of its own.
resetter = group if hasattr(group, "reset_seed") else sampler
try:
resetter.reset_seed(_sampler_seed(seed, names))
except Exception as error:
# Not just RuntimeError. The seed handed over is now 128 bits,
# which the legacy RandomState refuses with a ValueError, and a
# bare one of those does not say which sampler raised it.
# Broad: the seed is 128 bits, which legacy RandomState refuses
# with a ValueError that does not name the sampler.
raise RuntimeError(
f"An error occurred in the 'reset_seed' method of the "
f"CustomSampler for {', '.join(names)}"
Expand All @@ -627,9 +618,7 @@ def _validate_custom_sampler(self, input_name, sampler):
"""
Validate a custom sampler.

Seeding is not done here. It happens in ``_reset_custom_samplers``,
which runs in a fixed order because two samplers can share one
generator and whichever is reset last decides the stream.
Seeding happens in ``_reset_custom_samplers``, not here.

Parameters
----------
Expand All @@ -643,9 +632,8 @@ def _validate_custom_sampler(self, input_name, sampler):
AssertionError
If the input is not in a valid format.
"""
# Raised rather than asserted, the same way #1103 handles it: `python -O`
# strips an assert, and the documented AssertionError is kept so callers
# that already catch it still do.
# Raised, not asserted: `python -O` strips asserts. AssertionError is
# kept so callers that catch it still do. Same as #1103.
if not isinstance(sampler, CustomSampler):
raise AssertionError(
f"`{input_name}` must be a CustomSampler, not {type(sampler).__name__}"
Expand Down
Loading