Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to new numpy RNG API #308

Merged
merged 16 commits into from
Apr 24, 2024
Merged

Update to new numpy RNG API #308

merged 16 commits into from
Apr 24, 2024

Conversation

r-pascua
Copy link
Contributor

This PR implements a shift to use the new API for generating random numbers with numpy. Now, rather than seed the global numpy random state prior to simulating a component, users should provide a np.random.Generator instance that is seeded as they desire. For folks who prefer to use the Simulator, then they effectively don't need to change anything—the interaction with the Simulator looks the same in basically every case (there are some corner cases, but, assuming I'm not missing something major, they're unusual enough that they should just be addressed in one-off conversations).

The following simple rule can be applied to update code to be compliant with this change:

PRIOR TO THIS UPDATE

seed = 12345  # or whatever you want to use
np.random.seed(seed)
cmp = sim_component(*args, **kwargs)

AFTER THIS UPDATE

seed = 12345  # or whatever you want to use
rng = np.random.default_rng(seed)
cmp = sim_component(*args, **kwargs, rng=rng)

In both cases, args are the positional arguments required by the simulation component's call signature, while kwargs are the keyword arguments that are typically stored as the class instance's attributes (i.e., these are the model parameters).

The most substantial change here is how the Simulator juggles random states. This is hidden from the end user, but it is worthwhile to note the changes in some detail here for those interested in how the Simulator internals function. Here is a high-level description of how the logic worked prior to this update, as well as a brief description of how it works after the changes made in this PR.

PRIOR TO THIS UPDATE

Before simulating the desired effect with the add method, the Simulator checks to see if the user has requested that the random state be managed in a particular way by checking the value of the seed parameter. If the user provided something, then the Simulator effectively enters a random state management routine before evaluating the model. Importantly, the random state is set during this random state management routine via the old API np.random.seed(...). Seed caching is performed as-needed, depending on the type of seeding prescription requested.

AFTER THIS UPDATE

The initial logic is the same as before—the Simulator checks to see whether it should set the random state prior to evaluating the model. The main difference now is that the _seed_rng routine (the random state management routine mentioned above) returns both the (possibly updated) seed as well as the random number generator that has been seeded as desired. The seed returned is just to ensure that the Simulator has memory of what it was asked to do (which allows for cool stuff like making sure every baseline in a redundant group gets the same random seed, if desired), while the random number generator is passed along to the model and is ultimately used to make the random component of the model.

r-pascua added 11 commits April 23, 2024 15:14
The basic idea here is that the random state seeding is handled by the
Simulator, then the random number generator produced by the state
handling code is passed off to the SimulationComponent to be used in
simulating data. The changes added in this commit will not work until
the SimulationComponent children classes are updated to handle this
difference in how random number generation is handled.
This isn't a very elegant solution, since there's a one-liner clause
that pops up everywhere that has to do with all classes effectively
needing a rng attribute to play nice with the Simulator, but this should
work for now... I hope.
This class attribute helps the Simulator determine whether or not it
should be providing a given model with a random number generator when
calling _iteratively_apply
@r-pascua r-pascua requested a review from steven-murray April 23, 2024 23:52
Copy link

codecov bot commented Apr 23, 2024

Codecov Report

Attention: Patch coverage is 93.45794% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 93.49%. Comparing base (921110a) to head (f21b3a1).
Report is 1 commits behind head on main.

Files Patch % Lines
hera_sim/simulate.py 81.08% 5 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #308      +/-   ##
==========================================
+ Coverage   93.43%   93.49%   +0.06%     
==========================================
  Files          24       24              
  Lines        3243     3276      +33     
  Branches      711      712       +1     
==========================================
+ Hits         3030     3063      +33     
+ Misses        117      116       -1     
- Partials       96       97       +1     
Flag Coverage Δ
unittests 93.49% <93.45%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@steven-murray steven-murray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, great work @r-pascua, this looks really good.

hera_sim/foregrounds.py Show resolved Hide resolved
@steven-murray steven-murray merged commit 1752604 into main Apr 24, 2024
10 checks passed
@steven-murray steven-murray deleted the rng_update branch April 24, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants