epiverse-trace / finalsize

R package to calculate the final size of an SIR epidemic in populations with heterogeneity in social contacts and disease susceptibility
https://epiverse-trace.github.io/finalsize/
Other
11 stars 7 forks source link

Final size with risk groups using Newton solver #65

Closed pratikunterwegs closed 2 years ago

pratikunterwegs commented 2 years ago

This PR adds the Newton solver option to final_size_grps in R.

  1. Implements the Newton solver from https://gitlab.com/epidemics-r/code_snippets/-/blob/feature/newton_solver/include/finalsize.hpp in R, and fixes #55.
  2. Adds tests for the Newton solver:
    • Simple cases with synthetic data, fixes #56
    • Checks for failures at higher r0, fixes #57
    • Cases with multiple risk groups, fixes #58
    • Checks for failures with socialmixr::polymod data, fixes #59
  3. Adds documentation, fixes #60
  4. Adds solver argument to final_size_grps to allow users to choose between solvers, fixes #61, fixes #62
pratikunterwegs commented 2 years ago

Hi @Bisaloo, I'll update this PR with a rebase after you merge #63, any feedback here would be great. I imagine there are efficiencies to be found similar to the ones with the iterative solver. I'm running through the package again to check. @TimTaylor if you could take a look too that would be great.

pratikunterwegs commented 2 years ago

Adding a commit to correct the same r0 = 12 test, which I fixed in https://github.com/epiverse-trace/finalsize/pull/65/commits/30e34864e73e5e78ebb250846603edc3c4be2ccd for the iterative solver only

TimTaylor commented 2 years ago

Won't be able to review in near future so best select another.

pratikunterwegs commented 2 years ago

Updated branch with a local rebase on main after merging #63, there were conflicts in the test for the iterative solver with varying r0. I resolved these and force pushed the local to origin to avoid duplicate commits.

Windows build currently failing due to an update to curl on CRAN - likely to pass once curl binaries are available.

pratikunterwegs commented 2 years ago

Thanks @Bisaloo, I've changed how the solver option switches the solver function per your recommendation, and am now passing the solver options as a control list.

I removed the extra function arguments remaining from the C++ code - but also the p_susceptibility argument from the iterative solver and from the epi_spread return. The epi_spread function was returning a vector of 1.0s, which was being used in a multiplication step in solve_final_size_iterative - unnecessary. This means both solvers now take the same epi arguments; this is also newly documented.