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

Standardises generation of infections #214

Closed
wants to merge 3 commits into from
Closed

Conversation

seabbs
Copy link
Contributor

@seabbs seabbs commented Dec 30, 2020

This PR standardises the generation of infections and the use of covariates (currently GP and BP only). Work in progress.

@seabbs seabbs changed the base branch from master to update_growth December 30, 2020 22:20
@sbfnk
Copy link
Contributor

sbfnk commented Dec 31, 2020

Not having looked in detail can‘t this be simplified further by using the calculate_Rt function and separating out setting of the GP prior (whether on Rt or infections)?

@sbfnk
Copy link
Contributor

sbfnk commented Dec 31, 2020

I.e., can't we write the model as in #211 (comment)_ where R_t, I_t and r are all parameters? The only difference between the models is then in the prior choice, e.g. R_t could be a GP prior or an uninformed prior (i.e., a backcalculation model in which case it will just be calculated from uncertain I_t as in the first equation in the comment referenced), and this could be done in separate functions (and what's currently generate infections becomes calculate_Rt). Or am I missing something?

If this can be done, it would have the nice effects of, e.g., being able to use breakpoints/covariates on Rt in a model that has a GP prior on I_t and uninformed prior on R_t.

@sbfnk
Copy link
Contributor

sbfnk commented Dec 31, 2020

Looking again most of this is there already: all it would need is for all models to have R_t, r_t and I_t as parameters, and all models to use generate_infections_with_infectiousness (which I think should just be called renewal_model) whereas generate_infections_with_infectiousness would be re-named to reflect that all it does is set a prior on infections (which is all that determines infections if the priors on R_t and r_t are unconstrained - though in practice one might want to bound them an turn into uninformative proper priors even when not using a GP, e.g. uniform with some bound, that way their posteriors become proper).

Wouldn't change anything in practice but reduce code complexity and remove duplicate code (e.g. calculate_Rt vs generate_infections, where the former would no longer be needed if R_t was always a parameter).

@seabbs
Copy link
Contributor Author

seabbs commented Dec 31, 2020

Hmm so in my mind it was a choice (i.e infections directly, infections via renewal equation, infections via growth. It is not totally clear to me that you could do all of them at once as the renewal equation is iterative (in that it is forwards whereas when you calculate Rt from it it is backwards) and stan will try and fit all the parameters - will stare at the code a bit.

@sbfnk
Copy link
Contributor

sbfnk commented Dec 31, 2020

Hmmm, yeah so it might depend on whether Stan interprets an equals sign as a statement of equality or an assignment. If there's no way to have it interpret as equality than my suggestion won't work.

@seabbs
Copy link
Contributor Author

seabbs commented Dec 31, 2020

So my understanding of what it is doing is assignment - that could be incorrect however.

What I think would work would be something like this (where the infection and growth model are currently folded into a single function):

# set up covariates for each model
R_cov = update_covariate(...)
r_cov = update_covariate(...)
inf_cov = update_covariate(...)

# initialise unobserved infections using constant growth model
uobs_infs = generate_seed(...)
# generate infections using renewal 
infections = renewal_model(R_cov, uobs_infs,...)
infections = growth_model(infections, r_cov, ...)
infections = infection_model(constant = infections, inf_cov, ...)

# later as quantities
R = calculate_rt(infections, ...)
r = calculate_growth(infections, ...)

Where each later model using infections from the previous model as an input. That would let you have covariates/interventions on Rt and a GP on infections for example. This would add quite a bit of weight + code for a use case I am not entirely clear about.

The alternative would be the parallel approach (i.e they are options). So:

# set up covariate
cov = update_covariate(...)

# initialise unobserved infections using constant growth model
uobs_infs = generate_seed(...)
if (model == 1) {
  infections = renewal_model(cov, uobs_infs,...)
}else if (model ==2) {
  infections = growth_model( cov, uobs_infs ...)
}else if (model == 3) {
  infections = infection_model(constant, cov, uobs_infs, ...)
}

# later as quantities
R = calculate_rt(infections, ...)
r = calculate_growth(infections, ...)

@sbfnk
Copy link
Contributor

sbfnk commented Jul 27, 2022

Closing in favour of #307

@sbfnk sbfnk closed this Jul 27, 2022
@sbfnk sbfnk deleted the update_generation branch May 3, 2024 19:48
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