search

epiverse-trace / simulist

An R package for simulating line lists
https://epiverse-trace.github.io/simulist/
Other
4 stars 0 forks source link

Should initial value of `num_cases` be 1 in `.sim_internal()`? #109

Closed jamesmbaazam closed 2 months ago

jamesmbaazam commented 2 months ago

I was taking a quick look at the code for .sim_internal() and noticed that the stopping condition for the while loop num_cases is initiated at 0 (https://github.com/epiverse-trace/simulist/blob/925a43638170b2dc825751f41a681ad870093756/R/sim_internal.R#L33) but the model it calls (.sim_network_bp()) is seeded with 1 case (https://github.com/epiverse-trace/simulist/blob/925a43638170b2dc825751f41a681ad870093756/R/sim_network_bp.R#L45).

Should num_cases also start at 1?

joshwlambert commented 2 months ago

num_cases is initially set to 0 because the simulation (.sim_network_bp()) has yet to be called so there are no cases. Then the line you've linked to from .sim_network_bp() is setting up the simulation by infecting the index case.

num_cases needs to be initialised as 0 to ensure the simulation runs at least once. If the user specifies a minimum outbreak size of 1 and num_cases were initialised as 1 then the simulation would never be triggered. https://github.com/epiverse-trace/simulist/blob/925a43638170b2dc825751f41a681ad870093756/R/sim_internal.R#L36

jamesmbaazam commented 2 months ago

If the user specifies a minimum outbreak size of 1 and num_cases were initialised as 1 then the simulation would never be triggered.

That's true, although the solution would be to change the while loop condition to use <= instead of <.

joshwlambert commented 2 months ago

I agree that changing the while loop condition would give the same behaviour but given that this is not a functional change I will leave it as is.

If you think this could somehow lead to a bug please feel free to respond in this thread or make a PR.