-
Notifications
You must be signed in to change notification settings - Fork 9
Model builder, latent infection process components and tutorials #650
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
Conversation
…ents, 'aggregate' instead of 'jurisdiction'
…ents, 'aggregate' instead of 'jurisdiction'
for more information, see https://pre-commit.ci
… into mem_generic_observations
for more information, see https://pre-commit.ci
… into mem_generic_observations
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
… into mem_generic_observations
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @cdc-mitzimorris. Have now reviewed everything except the tutorials and tests. Flagged a bunch of small things and tried to be clear where changes are optional. My main big questions (open to pushback on both):
- Not sure specific distributional priors are needed / in scope. Currently prefer to show the user how to create a needed prior via
VectorizedRVin the tutorials. - TemporalProcess
RandomWalkshould wrap the process module's random walk rather than reimplementing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Suggest log y axis for all infection and admission plots
- Why not show the predicted latent (and/or posterior predictive) admissions on top of the observed?
- Would be nice for completeness to have a wastewater predicted/observed plot as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have now reviewed everything but tests. @cdc-mitzimorris I propose you make and/or push back all proposed changes and then I will re-review. I'd like to wait to review tests since some changes could require them to be reworked or all some to be eliminated (e.g. the hierarchical priors).
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
Co-authored-by: Dylan H. Morris <dylanhmorris@users.noreply.github.com>
…w into mem_hier_latent_processes
…n, remove fit(), remove sample() return - Rename baseline_temporal -> baseline_rt_process, subpop_temporal -> subpop_rt_deviation_process - Refactor RandomWalk temporal process to wrap pyrenew.process.RandomWalk - Replace custom PMF validation with validate_discrete_dist_vector - Remove MultiSignalModel.fit() in favor of Model.run() - Remove return value from MultiSignalModel.sample() (use Predictive/deterministic sites) - Remove unreachable lookback_days defensive check (enforced by ABC) - Update tests and tutorials Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
| n_processes=pop.K, | ||
| initial_value=jnp.zeros(pop.K), | ||
| n_processes=pop.n_subpops, | ||
| initial_value=jnp.zeros(pop.n_subpops), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To fix (not in this PR): initial deviations should be confiurable, not fixed.
dylanhmorris
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @cdc-mitzimorris!
|
thank you @dylanhmorris for your careful and patient review! |
Adds infrastructure for modeling infections across multiple subpopulations with hierarchical Rt structure, plus a builder pattern for composing multi-signal renewal models.
Key additions:
Architecture
Latent layer:
Model layer:
Suggested Review Order
- latent_hierarchical_infections.qmd: temporal process choices
- building_multisignal_models.qmd: full inference workflow
Review Scope
Feedback requested on:
Deferred to follow-up PRs:
noisea module rather than a submodule ofobservation#656, makes sense to address both locations consistentlyKnown Limitations / Future Work