Add epidemic model description vignette#188
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughA new R Markdown vignette is added documenting the ringbp epidemiological model. The vignette describes isolation, contact tracing, and quarantine interventions; transmission scenarios with varying presymptomatic transmission; model parameters including incubation distributions and delays; and references the scenario_sim() function for simulations. Changes
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
vignettes/ringbp-model.Rmd (5)
21-21: Please verify that relative figure paths work for installed vignettesYou are referencing schematics via relative paths such as
../man/figures/model_schematic_iso_prevent.svg. These will work when knitting from the package source, but can break once the vignette is installed (HTML moved intodoc/while images stay elsewhere). Consider either:
- using
knitr::include_graphics(system.file("figures", "model_schematic_iso_prevent.svg", package = "ringbp"))in a chunk, or- placing figures under
vignettes/(orinst/) in a layout known to work for installed vignettes and pkgdown.At minimum, please build and install the package locally and open the installed vignette to confirm all schematics render correctly.
34-35: Tidy wording in the contact tracing/quarantine descriptionMinor grammatical tweaks would improve readability here:
-In the scenarios that contacts are being traced in the model (i.e. the proportion of contacts ascertained > 0%), or quarantine is active, then the infectee is isolated at the earliest of the either the infectors isolation time or an onset-to-isolation delay after their symptom onset time. +In scenarios where contacts are being traced in the model (i.e. the proportion of contacts ascertained > 0%), or quarantine is active, the infectee is isolated at the earliest of either the infector's isolation time or an onset-to-isolation delay after their symptom onset time.This removes redundancy (“the earliest of the either”), fixes the possessive (“infector's”), and streamlines the sentence slightly.
44-45: Subject–verb agreement in description of scenarios (3a) and (3b)Very small grammar nit:
-Scenarios (3a) and (3b) shows when a symptomatic infector transmits to a symptomatic infectee. +Scenarios (3a) and (3b) show when a symptomatic infector transmits to a symptomatic infectee.This keeps the text polished without changing meaning.
66-67: Fix article in description of scenario (5b)Another small wording improvement:
-Scenario (5b) is equivalent to scenario (5), illustrating what happens if a infectee is not traced because contact tracing ascertainment is not 100%, but for (5b) the transmission is presymptomatic. +Scenario (5b) is equivalent to scenario (5), illustrating what happens if an infectee is not traced because contact tracing ascertainment is not 100%, but for (5b) the transmission is presymptomatic.
80-81: Clarify timing phrasing in scenario (8)Minor clarity/grammar suggestion:
-Scenario (8) shows the case where Person A infects Person B after Person A enters isolation, and Person B isolates a delay after becoming symptomatic. +Scenario (8) shows the case where Person A infects Person B after Person A enters isolation, and Person B isolates after a delay following symptom onset.This avoids the slightly awkward “isolates a delay after” phrasing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
man/figures/model_schematic_iso_infect.svgis excluded by!**/*.svgman/figures/model_schematic_iso_prevent.svgis excluded by!**/*.svgman/figures/model_schematic_scenario_postiso.svgis excluded by!**/*.svgman/figures/model_schematic_scenario_postsym.svgis excluded by!**/*.svgman/figures/model_schematic_scenario_presym.svgis excluded by!**/*.svg
📒 Files selected for processing (1)
vignettes/ringbp-model.Rmd(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: pkgdown
- GitHub Check: ubuntu-latest (oldrel-1)
- GitHub Check: macOS-latest (release)
- GitHub Check: ubuntu-latest (devel)
- GitHub Check: ubuntu-latest (release)
- GitHub Check: windows-latest (release)
- GitHub Check: ubuntu-latest (4.4)
|
Tip: it might be easier to read and review the vignette by pulling theses changes locally and running |
sbfnk
left a comment
There was a problem hiding this comment.
This is great, thank you! A couple of general comments:
- there's a mixture of active and passive voice for isolation ("person A isolates" equivalently to "person B is isolated") which is confusing for the reader; I'd suggest to streamline this to one of the two, probably "is isolated" given the focus on NPI / contact tracing rather than self-isolation
- "case" is not a great term for people - elsewhere you talk about infectors, infectees, persons or individuals which are all preferable
- it might be good to make the background of the figures white - as things stand they don't look great on dark background
Resolved in d545d0e.
Resolved in e74c127. |
e74c127 to
87654ba
Compare
Co-authored-by: Sebastian Funk <sebastian.funk@lshtm.ac.uk>
…odel_schematic_scenario_postiso
87654ba to
8c458fa
Compare
Co-authored-by: Sebastian Funk <sebastian.funk@lshtm.ac.uk>
|
All the comments are addressed and resolved, this is ready to be added to the merge queue. 🚀 |
pearsonca
left a comment
There was a problem hiding this comment.
A variety of minor notes. Broad item - {ringbp} vs {ringbp}? I mildly prefer latter.
As this is not related just to the changes in this PR I've opened an issue (#202) to discuss this with others and then I can implement the change throughout the package in a separate PR. |
This PR addresses #140 by adding a new vignette to the package (
ringbp-model.Rmd), and 6 images (.svg) with model schematics.The vignette includes descriptions and illustrations of how disease transmission and interventions are structured in the {ringbp} model.
The vignette explains each of the three invention types in the model: isolation, contact tracing and quarantine.
There are two types of model schematic included, one is a multi-transmission scenario and the other is a simpler single-transmission scenario that examine specific transmission and interventions scenarios that can occur in the {ringbp} model.
At the end of the vignette there is a link between the epidemiological parameters that control the interventions, disease transmissibility and delay distributions with the R code for parameterising a {ringbp} simulation. Lastly, there is a link to
outbreak_step()for readers that want to inspect the code implementing the scenarios outlined in the vignette.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.