epiverse-trace / epidemics

A library of published compartmental epidemic models, and classes to represent demographic structure, non-pharmaceutical interventions, and vaccination regimes, to compose epidemic scenarios.
https://epiverse-trace.github.io/epidemics/
Other
8 stars 2 forks source link

Interdependence of parameters within `epidemic()` #75

Closed TimTaylor closed 1 year ago

TimTaylor commented 1 year ago

Currently epidemic() has the following function signature:

epidemic(
  model_name = ("default", "vacamole"),
  population,
  infection,
  time_end = 200,
  increment = 1,
  ...
)

This means that both infection and ... are dependent on model_name and, as more models get added, users will have to move between multiple layers of abstraction / sets of documentation to understand what the function inputs.

I've not thought on it too much yet but my inclination is that it would be better to have dedicated functions for each model. I think this will be much clearer for end users and think it may also simplify some of the underlying code.

Thoughts?

pratikunterwegs commented 1 year ago

Thanks @TimTaylor - this issue completely passed me by.

My idea was that {epidemics} should have a minimal number of user-facing functions to reduce confusion and make the package less intimidating than one with many functions or models.

I broadly agree that users are likely to find it difficult to know what each model requires without doing some reading first. I had intended that users would take a look at the vignette for the model they want to use, which has fully worked out examples they could follow.

If you feel that having multiple model functions exported from {epidemics} is not likely to be an issue, then I can't think of any immediate reasons to not have one function, say, epidemic_default(), epidemic_XYZ(), for each model.

pratikunterwegs commented 1 year ago

Still, to answer your initial question, I'm not sure how the parameters are interdependent. A model call to "vacamole" would fail if the population argument did not have vaccinated compartments, so in that sense there's interdependence, but that's what we'd want, isn't it? The model-specific argument checker and preparation functions only specify minimum requirements on ... and other arguments.

Perhaps I haven't understood the question there. Having separate model functions could indeed allow more extensive documentation for each function, so not against removing this general wrapper. That needs to be weighed against the time cost of maintaining a larger namespace. Would be good to hear from potential users.

TimTaylor commented 1 year ago

Perhaps I haven't understood the question there. Having separate model functions could indeed allow more extensive documentation for each function, so not against removing this general wrapper.

Yes - that's what I think.

That needs to be weighed against the time cost of maintaining a larger namespace.

I think it will simplify the codebase by removing one layer of abstraction. I don't think there's really an additional time cost. The functionality remains and most namespace related stuff is automated anyway.

pratikunterwegs commented 1 year ago

Closing this as fixed by #86.