Combine simulate_tree() and simulate_tree_from_pop() into simulate_chains().

[jamesmbaazam]

This PR seeks to close #142 and fix #8 using ideas from #114, by combining simulate_tree() and simulate_tree_from_pop() into a single function. It does this by extending simulate_tree() to accept optional pop and percent_immune arguments that are used to track a susceptible population as an additional stopping criterion in the branching process simulations.

Branching processes are now simulated with an initial set of cases and an offspring distribution until no offspring can be produced because stat_max is reached or the susceptible population is completely exhausted.

• Please check if the PR fulfils these requirements

• I have read the CONTRIBUTING guidelines
• The commit message follows our guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

• What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

A feature.

• What is the current behaviour? (You can also link to an open issue here)

There are two separate functions for simulating solely from an initial number of cases (simulate_tree()) or from a susceptible population simulate_tree_from_pop()

• What is the new behaviour (if this is a feature change)?

The new simulate_chains() accepts index_cases as a required argument and optional pop and percent_immune arguments for creating a susceptible population to be tracked. When pop = Inf and percent_immune = 0, simulate_chains() is the same as simulate_tree()`.

• Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

Not necessarily.

• Other information:

I have increased the cyclocomp_linter limit to 25 to accommodate the changes here. I will link an issue to deal with reducing the complexity in simulate_chains().

[jamesmbaazam]

@sbfnk I've struggled to include the logic for dealing with the case where the total new cases > susceptible population given arbitrary offspring_dist. See my current implementation here on

epichains/R/simulate.r

Lines 215 to 218 in 5c007e1

	if (sum(next_gen) > susc_pop) {
	scaling_factor <- susc_pop / sum(next_gen)
	next_gen <- floor(next_gen * scaling_factor)
	}

. Could you help me there?

It is more straightforward in simulate_tree_from_pop() because offspring_dist is fixed as pois or nbinom and we can use their truncated distribution with a scaled mean that accounts for susceptibles, but here, I'm not sure how to go about it.

[sbfnk]

Great stuff! Just a few questions/suggestions for improvement.

Seems like some unrelated changes to the citation/roxygen/wodlist made it into this?

[jamesmbaazam]

Seems like some unrelated changes to the citation/roxygen/wodlist made it into this?

I added some references to the documentation of simulate_chains(), which introduced some failing spellchecks because of the author names, so I had to update the spelling list.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d499296) 99.11% compared to head (abc3915) 99.26%.
Report is 17 commits behind head on main.

❗ Current head abc3915 differs from pull request most recent head 8002054. Consider uploading reports for the commit 8002054 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #171      +/-   ##
==========================================
+ Coverage   99.11%   99.26%   +0.15%     
==========================================
  Files           8        8              
  Lines         562      684     +122     
==========================================
+ Hits          557      679     +122     
  Misses          5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sbfnk]

LGTM.

I've suggested a small change to the examples to make it clearer that not all arguments are required (would require re-rendering the docs, too, if going ahead with it).

[jamesmbaazam]

It's a good revision. Thanks, Seb.

[sbfnk]

Turns out my suggested commit was in the .Rd file - my bad! Needs transferring into the roxygen comments and re-rendering, then we're ready to merge I think.

[jamesmbaazam]

Turns out my suggested commit was in the .Rd file - my bad! Needs transferring into the roxygen comments and re-rendering, then we're ready to merge I think.

Yes, I later noticed too but fixed that. Have also implemented both suggestions from #179. Will set up a separate PR to deal with #179.