-
Notifications
You must be signed in to change notification settings - Fork 0
Immunity rework #24
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
Immunity rework #24
Conversation
… handles waning and multivalent vaccines, as well as flexible scheduling. While the waning does work as expected internally, it is not yet connected to the immunity model.
# Conflicts: # rotasim/rotasim_genetics.py
…o update severe chance.
…o update Rota class to always use the vaccine interventions.
…lasses because they're too inflexible
…ivery. The delivery class was too inflexible an incomplete, and this simplifies the code as well. Added tests for each of the new intervention features.
…recoveries is greater than number of infecteds
…d test_alt to match
daniel-klein
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @RHull-IDM, I added a few comments for your consideration. My quick review focused on the new interventions.py file rather than changes to rotasim or rotasim_genetics. Thanks!
| ppl = self.sim.people | ||
| if self.sim.ti in self.timepoints: | ||
| if self.sim.ti == self.timepoints[0]: | ||
| # if there is no strain associated with the product, select the most prevalent strain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in different sims (e.g. change rand_seed or other parameters), the vaccine might contain different antigens, based on prevalence at the first timepoint of the vaccine? This feels error-prone, consider requiring users to specify g and p.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This maintains the behavior of the original model. If @Suvanthee or Kyra or Alicia want to chime in I'm happy to change to whatever makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intended behavior was to
- Let the model reach an equilibrium for the first 5 years
- At 15 years pick the vaccine strain as the most prevalent strain in the last 10 years (skipping the first 5 years where the model is in the initialization phase)
Once the vaccine strain is picked we will not change it and use it in the future for both doses.
I think we should add the behavior where we skip the first 5 (time_to_equilibrium) years because at the start the simulation is very volatile similar to here.
For different seeds and other parameters we may indeed see different strains being picked. Our goal here was to study the effect of targeting the most prevalent strain though a vaccine in many settings. I think the current code achieves that.
An option that allows users to specify the strain would be a great addition too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suvanthee is correct that the original intention was to choose the most prevalent strain, even if that change from iteration to iteration. We asked Ryan to add the option to specify vx_strains so that we can maintain consistency in the vaccine strain across iterations as needed (e.g., if want to model specifically the effects of a G1P8 vaccine like Rotarix) - and expect that we will primarily use this functionality moving forward, but retain default behavior of selecting most prevalent strain if vx_strains are not specified.
| vx_intv_found = True | ||
| break | ||
|
|
||
| if not vx_intv_found: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to run a simulation without the RotaVaxProg?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was previously an integral part of the model and was not separate from the rest of the model logic, so I implemented it this way to maintain consistency. If there's a case for not using the vaccination program I will make it optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have this functionality. Earlier we had a vaccine_hypothesis where this was skipped. But I now realize that I got rid of it while removing the vaccine_hypothesis for simplifying the model
Suvanthee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @RHull-IDM This looks great. I am slowly going through the changes. left some comments.
| ppl = self.sim.people | ||
| if self.sim.ti in self.timepoints: | ||
| if self.sim.ti == self.timepoints[0]: | ||
| # if there is no strain associated with the product, select the most prevalent strain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intended behavior was to
- Let the model reach an equilibrium for the first 5 years
- At 15 years pick the vaccine strain as the most prevalent strain in the last 10 years (skipping the first 5 years where the model is in the initialization phase)
Once the vaccine strain is picked we will not change it and use it in the future for both doses.
I think we should add the behavior where we skip the first 5 (time_to_equilibrium) years because at the start the simulation is very volatile similar to here.
For different seeds and other parameters we may indeed see different strains being picked. Our goal here was to study the effect of targeting the most prevalent strain though a vaccine in many settings. I think the current code achieves that.
An option that allows users to specify the strain would be a great addition too.
rotasim/rotasim_genetics.py
Outdated
| ve_s = self.vaccine_efficacy_s_d2[pathogen_strain_type] | ||
| else: | ||
| raise NotImplementedError(f"Unsupported vaccine dose: {vaccine.dose}") | ||
| ve_s = vaccine.product.vaccine_efficacy_s[n_doses][pathogen_match] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the waning immunity should also affect the protection against severe infection. Previsously we only applied the severity reduction when the vaccine immunity has not waned (see if vaccine is not None and not vaccine.is_waned: above).
I think we need to apply the same reduction for the probability of getting a severe case as for vaccine immunity here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed - behavior should be to apply the same waning multiplicative factor to VE_i and VE_s (or wane total VE and then use the same ratio to calculate VE_i and VE_s at each time step)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added waning to the severity calculation.
rotasim/interventions.py
Outdated
|
|
||
| def breakdown_vaccine_efficacy(self, ve, x): | ||
| """ | ||
| Break down the vaccine efficacy into its components |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will send a description for this tomorrow.
| vx_intv_found = True | ||
| break | ||
|
|
||
| if not vx_intv_found: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have this functionality. Earlier we had a vaccine_hypothesis where this was skipped. But I now realize that I got rid of it while removing the vaccine_hypothesis for simplifying the model
|
Ended up going a different direction with a full refactor to work better with the latest Starsim version |
Updated vaccination intervention which now leverages the
starsim.Interventionclass.New features:
"2001-01-01"or2001.0Tests have been added for each of these features as well.
Closes #20
Note: I still need to update the baseline test values before merging.