Skip to content

testing latest epinow2 version#335

Closed
micahwiesner67 wants to merge 0 commit intomainfrom
dev-update_epinow2_vers_latest
Closed

testing latest epinow2 version#335
micahwiesner67 wants to merge 0 commit intomainfrom
dev-update_epinow2_vers_latest

Conversation

@micahwiesner67
Copy link
Copy Markdown
Collaborator

@micahwiesner67 micahwiesner67 commented Jul 7, 2025

Draft PR for CI purposes. Between the current version of EpiNow2 (1.6.1) and the latest non-dev version (1.7.1) some of the mechanics of how gp or horizon options are input has changed and thus code in the R/fit_model.R file needs to be adjusted accordingly. The documentation for EpiNow2 can help with this.

Initially, test-write_output.R had the most failing tests. This was due to the process_quantiles function now outputting an additional two columns (accumulate, breakpoint) which are not expected in the test-write_output.R tests. Also these columns contain NAs forcing other tests to also fail out.

Currently, we are passing all unit tests. An initial test run between current version (1.6.1) and latest version (1.7.1) shows slightly different results and version 1.7.1 shows diagnostic flags being triggered.
Update (7/15/25) this was partially improved by adjusting the rt prior sd to 1.0 from 0.2 (these are suggested defaults from EpiNow2 package)

Accomplished:

  1. Refactored to address issue in rt_opts
  2. Refactored to accomodate filter_leading_zeros new format in latest version
  3. Refactored forecast / horizon syntax
  4. Included "breakpoint" and "accumulate" (these are new columns added in 1.7.1.9000 dev version) in the process_samples and process_quantiles expected columns tests in test-write_output.R. Also necessary to remove these columns before testing columns for NAs as these populate with NAs
  5. Adjusted adapt_delta in gostic_fit to lower p_divergent under diagnostic_flag; Also, adjusted rt prior mean to 2 for gostic_toy_rt
  6. Switched confirm column in setup.R to use obs_incidence instead of incidence

Notes:
Default alpha_sd has been changed between versions (from 0.05 to 0.01)

Update (7/11/25): I diffed the fit.R file between version 1.6.1 and 1.7.1 and found few differences. I also diffed the opts.R file between the two versions and there are many areas of code which have been refactored, but none jump out to me as changing any default model parameters

Update (7/15/25):
Diagnostics extracted from a fit on the gostic_toy_rt data yields very similar results.
image

image

Diagnostics from re-running on 1.7.1 w/ new priors vs. 1.6.1 with old priors still shows 1.7.1 has more divergences (6 state:disease pairs, compared to 3 with 1.6.1s setup). After analyzing these diagnostic files, none of the state:disease pairs with divergences have other diagnostic_flags (none have high_rhat or exceeded max_treedepths)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 7, 2025

Thank you for your contribution @micahwiesner67 🚀! Your pkgdown-site is ready for download 👉 here 👈!
(The artifact expires on 2025-07-22T17:54:51Z. You can re-generate it by re-running the workflow here.)

@SamuelBrand1
Copy link
Copy Markdown

SamuelBrand1 commented Jul 9, 2025

Hi @micahwiesner67 .

Looking at this draft PR I think you've correctly converted the functionality of the fit_model functions from EpiNow2 1.6.1 to 1.7.1, in the sense that the same probabilistic model is defined by fit_model in main vs this branch. A potential gotcha is default priors changing between 1.6.1 and 1.7.1 but I'm not seeing evidence for this in the News.md (except possibly a rescaling which I'm asking about).

Out-of-scope comments

However, there are quite a few features that would affect model performance that can't be varied by fit_model e.g.:

  • Prior for GP length scale is stuck at epinow2 default; no interface to ls = LogNormal(mean = 21, sd = 7, max = 60), in gp_opts.
  • There is no interface to the obs model, which seems to be hard coded to be default.
  • The generative model for Rt is default frozen at the end of data horizon (this will affect any forecast eval); no interface to rt_opts future kwarg.
  • The generative model for Rt is default frozen as a GP on the $\log(R_t)$ differences rather than a GP on deviations from a global (log) mean $R_0$ value; no interface to rt_opts gp_on kwarg. IIRC @kgostic was interested in one point about trying mean-reverting $R_t$ processes?

These are out-of-scope of this PR, but I was wondering where a good place to log this as a issue would be (either on this pipeline repo or elsewhere)?

@SamuelBrand1
Copy link
Copy Markdown

A potential gotcha is default priors changing between 1.6.1 and 1.7.1 but I'm not seeing evidence for this in the News.md (except possibly a rescaling which I'm asking about).

epiforecasts/EpiNow2#1109 (comment)

The meaning of the length scale prior hasn't changed, its just used more efficiently under-the-hood.

@micahwiesner67
Copy link
Copy Markdown
Collaborator Author

Feel free to log these out-of-scope thoughts in an issue in this repo. Thanks for looking into this!

Comment thread tests/testthat/setup.R Outdated
Comment thread tests/testthat/setup.R Outdated
Comment thread tests/testthat/test-write_output.R Outdated
Copy link
Copy Markdown
Collaborator

@kgostic kgostic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some high-level comments! Broadly this looks like it will work. Let's chat f2f about whether you're stuck on anything.

@SamuelBrand1
Copy link
Copy Markdown

This looks good in terms of reading the src but noting that you're still chasing

version 1.7.1 shows diagnostic flags being triggered

So probably good idea to avoid merge until we have clarity on those.

@micahwiesner67 micahwiesner67 force-pushed the dev-update_epinow2_vers_latest branch from b79d528 to dab704a Compare August 14, 2025 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants