Shrink finalsize to single function

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

Shrink finalsize to single function #73

Closed pratikunterwegs closed 2 years ago

pratikunterwegs commented 2 years ago

This PR compresses the finalsize package down to the single function final_size, which:

Allows choosing a solver from "iterative" or "newton"
Provides C++ solvers for final size calculations as internal Rcpp functions, and removes the R solvers,
Removes the R implementations of the functions for final_size, final_size_grps, and epi_spread.
Removes the standalone Rcpp function final_size_grps_cpp,
Fixes #74, returns a dataframe of the proportion infected (the final size) per age and risk group

pratikunterwegs commented 2 years ago

This PR is ready to merge given the original aim of shrinking finalsize down to a single function. The current development pipeline seems to also need to include #72 and #74 into this PR, and so this PR is being converted into a draft again.

pratikunterwegs commented 2 years ago

This PR is being made live again as #72 has been closed, and this PR fixes #74.

pratikunterwegs commented 2 years ago

Commit https://github.com/epiverse-trace/finalsize/pull/73/commits/8ff99f03389db59c5339053798c38b677bc78b3d corrects output to be the proportion of each age-and-risk group to be infected. The previous behaviour was to return a _pinfected which was always, at most, the same as the proportion of each age group i in risk group j. The current behaviour allows the full range of 0.0 - 1.0 to be returned for each age-risk group combination.

Thanks to @joshwlambert for helping with this!

joshwlambert commented 2 years ago

Reviewed the changes to the code with @pratikunterwegs which was useful to fully understand the changes that have been implemented. A few other suggestions which are more personal preference:

when mismatching the solver option and control list (e.g. control list from iterative but specifying newton) the error message is not very informative. Could possibly input check the names or length of the list matches the solver type. Another option is a function (e.g. create_solver_control() which is a trivial function that checks the inputs against the solver specified and makes sure they are of the correct type and have the correct names).
the final_size() function fails when iterations are too low, but runs without error when the tolerance is very high (which I assume gives inaccurate p_infected estimates), could catch this at input checking if there is a threshold above which results become erroneous.

pratikunterwegs commented 2 years ago

Thanks @joshwlambert and @Bisaloo for your comments. Could one of you approve this please so I can get it merged.