-
Notifications
You must be signed in to change notification settings - Fork 40
Improve identifiability of non-stationary GP #1396
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
Draft
sbfnk-bot
wants to merge
16
commits into
main
Choose a base branch
from
identifiable-non-stationary-gp
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
5d870b5
Improve identifiability of non-stationary GP
sbfnk-bot 3d96f3f
Widen default alpha to compensate for rescaled GP
sbfnk-bot f25229e
Update documentation
epiforecasts-workflows[bot] e0455bc
Widen default alpha further to match observed posterior
sbfnk-bot 08d5717
Merge branch 'identifiable-non-stationary-gp' of github.com:epiforeca…
sbfnk-bot 965d38c
Update gp_opts.Rd for new alpha default
sbfnk-bot e234216
Update tests for new GP parameterisation
sbfnk-bot 71112fd
Widen default alpha to Normal(0, 0.5) and document weak identification
sbfnk-bot 7a0326e
Revert default alpha to Normal(0, 0.2)
sbfnk-bot 7a3d957
Strip PR back to centring-only
sbfnk-bot f00a6a7
Merge branch 'main' into identifiable-non-stationary-gp
sbfnk be45c1b
address review: document new R0 interpretation under centred GP
sbfnk-bot 674ec6f
Merge branch 'identifiable-non-stationary-gp' of github.com:epiforeca…
sbfnk-bot 0055390
address review: warn on custom prior under centred GP
sbfnk-bot d5f9081
Apply user prior on derived initial Rt under centred GP
sbfnk-bot 3351aa5
fix ci: provide empty init prior data items in simulate_infections
sbfnk-bot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
this is fine but it changes the interpretation of the prior passed to
rt_opts()-- this needs to be reflected in the documentation in various places (also the model description vignette I think), and should probably have an argument rename frominitand a deprecation cycle.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.
It also makes it much hard to set. I wonder if we can keep it with its current defintion but also keep this change? A prestan transform of some kind?
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.
Addressed in be45c1b:
rt_opts()priordoc and the three estimate_infections vignettes (workflow, options, math description) now reflect that with the default non-stationary GP,R0is the mean Rt over the observation window rather than the initial Rt. Stationary GP and no-GP cases keep the initial-Rt meaning.On the argument rename + deprecation: the current argument is
prior(notinit) and is generic enough that I don't think the name itself needs to change — the meaning shift is captured in the docs. Happy to do a rename (e.g.prior→r_mean_priorwithlifecycle::deprecate_warn()) if you'd prefer, just want to confirm the new name before doing the cycle. Could you point me at what you had in mind?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.
we might not need a rename but if we change the interpretation of the parameter we need, at the minimum, a warning to anyone who sets this to something other than the default.
What about @seabbs's comment? It's often easier to set a prior for initial R than mean R. Could this behaviour be recovered under the updated cumsum centering?
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.
Added the warning in 0055390:
rt_opts()now emits acli_warn()whenever a user supplies a non-defaultpriorwhilegp_on = "R_t-1"(the default), explicitly noting the mean-Rt interpretation. Stationary GP (gp_on = "R0") and no-GP paths don't trigger it.On @seabbs's question about recovering the initial-Rt semantics with a pre-stan transform: I worked through the algebra and it doesn't separate cleanly. The geometric improvement from centring comes from imposing
mean(gp) = 0. If you also imposegp[1] = 0(so thatR0 = R[1]), thengp_for_logR = gp_centred - gp_centred[1] = cumsum(noise)— the centring cancels out and you're back to the original (bad-geometry) parameterisation. Any single linear constraint on the trajectory removes one degree of freedom from the (R0, drift) ridge; choosingmean(gp) = 0and choosinggp[1] = 0are two different choices of the same kind of constraint, and they produce the same likelihood but a different identification for R0.So the only way to keep the initial-Rt interpretation is the original parameterisation, which has the ridge. We have to pick one. The warning is the honest minimum here.
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.
Better answer landed in d5f9081: the user-facing interpretation is preserved (
rt_opts(prior=...)is once again the prior on the initial Rt). No warning, no rename, no doc shift.Implementation: R0 now leaves the generic params vector.
R_mean(the trajectory mean) is sampled internally — that gets the geometry win from centring. The user's prior is applied to the derived initial Rt R[1] via a newcentred_gp_init_lpdfStan helper, with a Jacobian correction (just a +log(R[1]) term) so that the resulting joint prior over (R[1], noise, R[t]) matches main exactly. Change of variables, determinant 1.The plumbing (
init_param_ids,init_dists,init_dist_paramsdata items +pack_init_prior()R helper +centred_gp_init_lpdfStan helper) is intentionally parameter-agnostic — designed as a prototype for the time-varying-parameter framework in #600. Today R0 is the only entry; adding a time-varying alpha later is a one-line dispatch branch in Stan plus appending to the data items.Verified end-to-end on the previously catastrophic seed=8 (R-hat=6.10 on main): td=0, div=1, R-hat=1.005, ESS=663. R[1] posterior mean=2.29 (user prior on initial Rt → posterior on R[1]); R_mean[1] posterior mean=1.00 (data-determined trajectory mean). The two quantities are distinct and the user's prior lands where intended.