Skip to content
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

Improve the prior interface for make_epi_model_inference #45

Closed
seabbs opened this issue Feb 19, 2024 · 3 comments · Fixed by #61
Closed

Improve the prior interface for make_epi_model_inference #45

seabbs opened this issue Feb 19, 2024 · 3 comments · Fixed by #61
Assignees
Labels

Comments

@seabbs
Copy link
Contributor

seabbs commented Feb 19, 2024

As the title says we should be able to improve the prior interface. The simplest improvement would be to generalise the argument name for the latent process priors to reflect that it is used across all submodules. An additional improvement would be to think more deeply about the coupling of priors to model submodules.

This issue was discussed in #44

@SamuelBrand1
Copy link
Collaborator

The simplest improvement would be to generalise the argument name for the latent process priors to reflect that it is used across all submodules.

Yes, atm I just use merge on the NameTuples of each set of prior distributions.

An additional improvement would be to think more deeply about the coupling of priors to model submodules.

I'm going to try and prototype this with the easy fix as back up. It shouldn't be very hard. Basic idea is to wrap the function that makes a Turing model for each process with its priors in a struct. If you think thats cleaner than having a NamedTuple of priors lets do that.

The essential question is:

  • Should all the priors "live" together in code as one object?
  • Should the priors of a particular process "live" with the process definition as one object?

@seabbs
Copy link
Contributor Author

seabbs commented Feb 19, 2024

I'm going to try and prototype this with the easy fix as back up. It shouldn't be very hard. Basic idea is to wrap the function that makes a Turing model for each process with its priors in a struct. If you think thats cleaner than having a NamedTuple of priors lets do that.

Yes I think this is a nice idea.

Should the priors of a particular process "live" with the process definition as one object?

This seems cleanest to me and I think likely to have fewer failure states.

@SamuelBrand1
Copy link
Collaborator

My proposal is to create a struct to contain both the Turing model and its priors; this will get passed to the overall Turing model constructor.

Proposed new model diagram

flowchart LR

    A["Underlying dists.
and specify length of sims
---------------------
EpiData"]

    B["Choice of target
for latent process
---------------------
DirectInfections
    ExpGrowthRate
    Renewal"]

C["Observational Data
---------------------
Obs. cases y_t"]
D["Latent processes
---------------------
random_walk"]
E[Turing model constructor]
F["Latent Process priors"]
G[Posterior draws]
H[Posterior checking]
I[Post-processing]
DataW[Data wrangling and QC]
J["Observation models
---------------------
delay_observations"]
K["Observation model priors"]

A --> EpiModel
B --> EpiModel
EpiModel -->E
C-->E
D-->LatentProcess
F-->|default_rw_priors|LatentProcess
J-->ObservationModel
K-->|default_delay_obs_priors|ObservationModel
LatentProcess-->E
ObservationModel-->E
E-->|sample...NUTS...| G
G-.->H
H-.->I
DataW-.->C
Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants