Closed pratikunterwegs closed 10 months ago
Thanks @jamesmbaazam for the review, I'll take a look tomorrow and aim to get it merged by the start of next week.
Thanks @jamesmbaazam, that was helpful. I've merged most of your suggestions, and taken most of the others too.
Thanks, @pratikunterwegs. These are nice changes that will introduce some flexibility in the ebola model. I have left line comments. Below, I'm leaving other comments that could not go on lines that were not changed.
- In general, the state names vector
c( "susceptible", "exposed", "infectious", "hospitalised", "funeral", "removed" )
is repeated in several places in the code. It would be better to define it once and then use it everywhere.For example, if defined as
states <- c( "susceptible", "exposed", "infectious", "hospitalised", "funeral", "removed" )
then
L290 - probably safer to use
length(states)
in case the initial state changes in the future.L291 - probably safer to do
colnames(sim_data) <- states
Thanks, I've defined the compartment names in the function body and reused them for names and lengths
- Would be nice to have one or two quick examples in the function documentation. You could provide examples with and without time
dependence just to reinforce the idea in the user.
Broadly agree, however the setup steps for these models are pretty long and are better shown as vignettes to clearly explain setup.
- For the doc of the
time_dependence
argument, please link to
the time_dependence.Rmd vignette.
Linked from all instances of this argument.
- In the Ebola vignette, I'm a bit confused about why the scenarios with intervention tend to have higher outbreak sizes than the baseline as they take off. I would have expected the opposite.
My guess is that this is an artefact, not a real effect. The interventions mostly take a while to have any effect on the epidemic size.
- More broadly, it would be nice if you could unify the interface by finding a way around applying the
<vaccination>
class to these types of discrete-time models. It would remove any kind of confusion in using time-dependence to model vaccination in Ebola but<vaccination>
to model vaccination in the other models.
This could be done although it would take some time as the vaccination objects are optimised for the ODE models. More relevant though is that this model doesn't have a vaccinated compartment, as this is not included in Li et al.'s consensus model. We can consider adding one, but my impression is that EVD outbreaks are expected to end before immunity due to vaccination becomes really important.
More relevant though is that this model doesn't have a vaccinated compartment, as this is not included in Li et al.'s consensus model.
That's true. I forgot about that.
We can consider adding one, but my impression is that EVD outbreaks are expected to end before immunity due to vaccination becomes really important.
Reactive ring vaccination is possible. Conceptually, these individuals will not partake in transmission dynamics because they are removed into the absorbing state (here, the removed class) and could contribute to herd immunity.
Reactive ring vaccination is possible. Conceptually, these individuals will not partake in transmission dynamics because they are removed into the absorbing state (here, the removed class) and could contribute to herd immunity.
Sure, thanks - I would say that that adding that functionality might require building off a different kind of model which is already present in {epichains} or {ringbp}? Happy to include once the scope of the different packages is decided.
This PR contains further fixes to the ebola model:
prop_hospitalised
,etu_safety
andfuneral_safety
to parameters representing risk, so that<rate_intervention>
s can be used to reduce these values without allowing #109,