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

Outcome of test in tests_newborn_outcomes depends on RNG state #274

Closed
matt-graham opened this issue May 18, 2021 · 6 comments · Fixed by #275
Closed

Outcome of test in tests_newborn_outcomes depends on RNG state #274

matt-graham opened this issue May 18, 2021 · 6 comments · Fixed by #275
Assignees

Comments

@matt-graham
Copy link
Collaborator

While reviewing #267 @asif picked up a use of the global NumPy random number generator interface numpy.random rather than the local seeded RandomState in newborn_outcomes

birth_weight = np.random.normal(loc=params['mean_birth_weights'][mean_birth_weight_list_location],
scale=standard_deviation)

After changing this to

self.rng.normal(loc=params['mean_birth_weights'][mean_birth_weight_list_location],                                 
                         scale=standard_deviation)

the test_newborn_hsi_applies_risk_of_complications_and_delivers_treatment_to_facility_births test in test_newborn_outcomes fails at this line

assert sim.population.props.at[child_id, 'nb_early_onset_neonatal_sepsis']

From what I can tell from digging through the code in newborn_outcomes , this property is set in

# The linear model calculates the individuals probability of early_onset_neonatal_sepsis
if self.eval(params['nb_newborn_equations']['early_onset_neonatal_sepsis'], child_id):
self.newborn_complication_tracker['early_onset_sepsis'] += 1
df.at[child_id, 'nb_early_onset_neonatal_sepsis'] = True
logger.debug(key='message', data=f'Neonate {child_id} has developed early onset sepsis following delivery')

based on the outcome of a call to NewbornOutcomes.eval which uses a linear model to compute the probability for a Bernoulli outcome and then uses the pseudo-random number generator (RNG) to compute the outcome. This means that the value set for the nb_early_onset_neonatal_sepsis is dependent on the state of the RNG which would explain why this test fails when updating to use the module level RNG.

It seems like this might potentially also be related to this previous PR #260

@tamuri @tbhallett @joehcollins - tagging you all as you are all listed as authors on the commit which added the test

@matt-graham matt-graham changed the title Outcome of tests in tests_newborn_outcomes depends on random number generator state Outcome of test in tests_newborn_outcomes depends on RNG state May 18, 2021
@tamuri
Copy link
Collaborator

tamuri commented May 18, 2021

Please take a look at this @joehcollins

@joehcollins
Copy link
Collaborator

Hi @matt-graham @tamuri - thanks for catching this. Thought i'd forced nb_early_onset_neonatal_sepsis == True in this test by setting the intercept value of the model to 1 but some lurking treatment effects reduced the probability to below one (but close enough that even a couple of runs with a different seed didnt pick it up)- if that makes any sense.

I've made changes in a separate branch, including the change to self.rng.normal, shall I submit as a PR @tamuri?

@tamuri
Copy link
Collaborator

tamuri commented May 18, 2021

Thanks, Joe. If you let Matt know the branch, he'll merge your changes into this PR.

@joehcollins
Copy link
Collaborator

Perfect just running the test file once more

@joehcollins
Copy link
Collaborator

@matt-graham - everything seems to be working ok now. The branch is jcollins_rng_fix_may2021. Thanks!

@matt-graham
Copy link
Collaborator Author

matt-graham commented May 18, 2021

Thanks @joehcollins for the quick response and fix. I've merged your branch into my branch for #267. Once that gets merged we can close this issue too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants