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

How to use epidemics-classes with {odin} models #60

Merged
merged 9 commits into from
Jul 24, 2024

Conversation

pratikunterwegs
Copy link
Contributor

@pratikunterwegs pratikunterwegs commented May 30, 2024

This draft PR aims to fix #54 and is a work in progress. It builds off the update-renv branch and needs to be rebased on main before merging. This is a trial and may or may not be merged depending on whether {epidemics} features can be used with {odin} models.

@pratikunterwegs
Copy link
Contributor Author

Unsure why checks are failing as the scripts have been updated to use epiparameter::epidist_db().

@pratikunterwegs
Copy link
Contributor Author

Workflow failing as {rlang} v1.1.4 binaries may not yet be ready - will rerun later in the week to check.

Copy link
Member

@avallecam avallecam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One review comment related to #43, which is now solved:

  • We just added an setup chunk to template and all entries.
  • We added the rationale behind this in the Code style section in the CONTRIBUTING guidelines

@pratikunterwegs pratikunterwegs marked this pull request as ready for review June 17, 2024 08:43
Copy link
Member

@adamkucharski adamkucharski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together. This is a useful addition showing how epidemics objects can fit together within odin methods, and open that ecosystem in the process.

I've also made a suggestion for how a vector input of age-specific reduction can make it feasible to incorporate the school closure intervention object already defined in {epidemics} README.

@adamkucharski
Copy link
Member

Was exploring the odin model more and noticed that interventions seem to have different effect in odin and original epidemics implementation (despite the two outputs matching in the absence of interventions once model was SEIR - see this branch).

If R0=1.5, then reducing contacts across ages by 33% should cause the epidemic to roughly level off (because R = 1.5*(1-0.33) = 1). Or just under 33% given some immunity has already accumulated. This is what happens with the {odin} implementation. However, in the {epidemics} README implementation, the epidemic seems to level off when the contact reduction is around 15%, suggesting that the effect of interventions may be inflated somewhere in the model code. Will also flag as potential issue on {epidemics}.

@adamkucharski
Copy link
Member

Note the above issue has been resolved as a diffference in assumptions about how interventions acting on contacts (i.e. acting on both contacts to and from). The further-odin-development branch now has an {odin} implementation that matches the {epidemics} README, so might be worth merging that with this PR

@avallecam
Copy link
Member

avallecam commented Jul 24, 2024

this is ready to merge 🚀

We'll continue the review in an upcoming PR from further-odin-development branch. These include:

  • combine isolated chunks to only one code chunk ready to make this ready to copy and paste for user
  • add the warning and message FALSE to this chunk
  • add question mark to entry name
  • add options(odin.verbose = FALSE) to setup chunk

@avallecam avallecam merged commit d410d3c into epiverse-trace:main Jul 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Show {epidemics} model with {odin} backend
3 participants