Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split up package #338

Closed
sbfnk opened this issue Nov 24, 2022 · 2 comments
Closed

Split up package #338

sbfnk opened this issue Nov 24, 2022 · 2 comments
Labels
question Further information is requested

Comments

@sbfnk
Copy link
Contributor

sbfnk commented Nov 24, 2022

There might be a case for splitting up the package into multiple smaller ones, which could have benefits to usability, developer engagement and future sustainability. For example we could have a "core package" centered around implementing estimate_infections and the corresponding model, and the following related packages:

  1. a package implementing the truncation model from estimate_truncations - this would facilitate replacing this by improved approaches such as in epinowcast or dynamicaltruncation that could still feed into estimate_infections
  2. a package implementing the estimate_secondary model
  3. a package for the more engineered versions epinow and regional_epinow facilitating running on HPCs / logging etc.
  4. a package for visualisation, which could be based on a base class that could eventually also be provided by other packages that perform similar tasks, and maintain utility once all functionality provided here is superseded by functionality in epinowcast

A downside of at least the first two points (splitting off models) is that some common Stan code e.g. for the PMFs or convolutions would get lost unless this was e.g. provided as a git submodules. There would also need to be some common functionality (possibly provided by the core package) e.g. on processing arguments/stan options etc. Points 3 and 4 might still be worth considering even if we decide against proceeding with 1 and 2.

@sbfnk sbfnk added the question Further information is requested label Nov 24, 2022
@sbfnk
Copy link
Contributor Author

sbfnk commented Nov 28, 2022

Point 2 would of course be obsolete if #313 was addressed.

@seabbs
Copy link
Contributor

seabbs commented Jan 18, 2023

Yes, I agree.

In terms of other parts of the package functionality that people make use of I think there is the following:

  1. Censored delay distribution estimation
  2. Parameter estimates storage and retrieval
  3. Visualisation of multiple models (i.e the output from regional_ functions).
  4. Splitting estimate_infection still further into the deconvolution and generative process approaches
  5. The adjust_ family of functions.

Of these the functionality in 1. isn't anything that special. From a more methodological point of view the work I have been doing in dynamicaltruncation supercedes it. It also looks like the quickfit package being built as part of Epiverse trace will also replicate some of this functionality and provide a likely faster/easier user experience.

  1. Has essentially already been superseded by another Epiverse project (epiparameter) which follows a very similar design methodology to that implement here and so could be a slot in replacement should this functionality be dropped.

  2. Could form part of some kind of base case visualisation package but may be better suited to another package explicitly designed for multiple estimates. Not sure there is a drop-in for this as of yet.

  3. This is a tricky one as quite a lot of code is shared but doing so would drastically reduce technical debt. It would also help surface the deconvolution approach which is likely attractive for people aiming for speed vs other factors in their pipelines.

  4. These existed as tools for our originally multiple-step forecasting functionality (which forecast $R_t$ and then simulated infections. Tools available at the time (for example projections) were not suitable for this and so we rolled our own. However now these are not used here but still remain useful for others potentially (hence not dropping them). These would also fit nicely into another package.

@epiforecasts epiforecasts locked and limited conversation to collaborators Jan 25, 2023
@seabbs seabbs converted this issue into discussion #352 Jan 25, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants