Add Stan support as alternative Bayesian modeling backend - #175
Add Stan support as alternative Bayesian modeling backend#175d-morrison with Copilot wants to merge 57 commits into
Conversation
Co-authored-by: d-morrison <2474437+d-morrison@users.noreply.github.com>
Co-authored-by: d-morrison <2474437+d-morrison@users.noreply.github.com>
Co-authored-by: d-morrison <2474437+d-morrison@users.noreply.github.com>
d-morrison
left a comment
There was a problem hiding this comment.
fix failing workflow checks
d-morrison
left a comment
There was a problem hiding this comment.
address workflow check failures
…s.yml Co-authored-by: d-morrison <2474437+d-morrison@users.noreply.github.com>
Co-authored-by: d-morrison <2474437+d-morrison@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
- Set nBurnin attribute to nadapt in run_mod_stan() output, since ggmcmc::ggs() always reports 0 for Stan (warmup excluded upstream) - Remove noisy unconditional cli_inform() for multi-strata in sample_predictive_stan(); consistency is already validated at fit time - Expand @returns in prep_data_stan() to document all list elements and the zero-padding convention (vs NA in JAGS output) - Update Rd files to match Co-Authored-By: claude[bot] <claude[bot]@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The Stan backend uses weakly-informative defaults for prec_hyp_param (c(1, 1/9, 1, 1/9, 1)) that differ from JAGS (c(1, 1e-5, 1, 0.001, 1)) for HMC stability. Update: - Snapshot to match new defaults - @PARAM docs in prep_priors_stan to list the Stan-specific defaults - Example to use Stan defaults instead of JAGS values
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| // output of prep_priors_stan(): mu_hyp, prec_hyp, omega, wishdf, | ||
| // prec_logy_hyp), so run_mod() (JAGS) and run_mod_stan() (Stan) fit the same | ||
| // model and their posteriors can be compared directly. I derive the HMC | ||
| // reparameterizations in the companion note model_stan_transform_from_jags.md. |
There was a problem hiding this comment.
Broken reference — hallucination risk: model_stan_transform_from_jags.md does not exist in this repository. Searching the whole tree finds no such file. The companion derivation lives in vignettes/articles/jags-to-stan.qmd. Please update both this line and line 76 (same reference) to point to the real document.
| // reparameterizations in the companion note model_stan_transform_from_jags.md. | |
| // reparameterizations in the companion vignette vignettes/articles/jags-to-stan.qmd. |
|
|
||
| transformed data { | ||
| // Moment-match the JAGS inverse-Wishart covariance prior to an LKJ + | ||
| // lognormal factorization (derivation in model_stan_transform_from_jags.md). |
There was a problem hiding this comment.
Same broken reference — model_stan_transform_from_jags.md does not exist. Update to reference vignettes/articles/jags-to-stan.qmd.
| // lognormal factorization (derivation in model_stan_transform_from_jags.md). | |
| // Moment-match the JAGS inverse-Wishart covariance prior to an LKJ + | |
| // lognormal factorization (derivation in vignettes/articles/jags-to-stan.qmd). |
| { | ||
| one_minus_shape <- 1 - shape_pop | ||
| # Compute the argument to log, ensuring it stays positive | ||
| log_arg <- y1_pop^one_minus_shape - | ||
| one_minus_shape * alpha_pop * (t - t1_pop) | ||
| # Use pmax to ensure log_arg > 0 (avoid NaN from log of negative) | ||
| (1 / one_minus_shape) * log(pmax(log_arg, .Machine$double.eps)) | ||
| } |
There was a problem hiding this comment.
Numerical instability near shape ≈ 1: this formula is algebraically equivalent to the Stan model's log_two_phase function but less numerically stable. When shape_pop is close to 1 (i.e. one_minus_shape ≈ 0), log_arg → 1 and log(1)/one_minus_shape → 0/0, producing NaN for those posterior draws. The pmax() guard prevents log(negative) but not the 0/0 singularity.
The Stan model avoids this with log1p, which handles the a → 0 removable singularity smoothly (see the functions {} block in model.stan). Use the same formula here:
| { | |
| one_minus_shape <- 1 - shape_pop | |
| # Compute the argument to log, ensuring it stays positive | |
| log_arg <- y1_pop^one_minus_shape - | |
| one_minus_shape * alpha_pop * (t - t1_pop) | |
| # Use pmax to ensure log_arg > 0 (avoid NaN from log of negative) | |
| (1 / one_minus_shape) * log(pmax(log_arg, .Machine$double.eps)) | |
| } | |
| { | |
| # Use the numerically stable log1p form that matches model.stan's | |
| # log_two_phase(): log_y1 - log1p(a*alpha*(t-t1)*y1^a) / a | |
| # Stable as a = shape-1 -> 0 (no 0/0 singularity). | |
| a <- shape_pop - 1 # > 0 since shape > 1 | |
| log(y1_pop) - log1p(a * alpha_pop * (t - t1_pop) * y1_pop^a) / a | |
| } |
|
|
||
| # Attach antigens and ids attributes for sample_predictive_stan() | ||
| attr(stan_out, "antigens") <- attr(longdata, "antigens") | ||
| attr(stan_out, "ids") <- attr(longdata, "ids") |
There was a problem hiding this comment.
Misleading ids attribute in stratified runs: longdata here is always from the last stratum processed, so attr(stan_out, "ids") will only contain the last stratum's subject IDs, not the full set. While sample_predictive_stan() currently ignores this attribute (it only reads antigens), anyone who inspects attr(result, "ids") after a stratified run will get an incomplete list.
Consider either (a) accumulating all IDs across strata:
all_ids <- c(all_ids, attr(longdata, "ids")) # inside the loop
# ...
attr(stan_out, "ids") <- all_idsor (b) setting the attribute to NULL / documenting clearly that it only reflects the last stratum, to prevent silent misuse.
|
Claude finished @Kwan-Jenny's task in 5m 16s —— View job PR #175 Code Review: Add Stan support as alternative Bayesian modeling backend
Status: 3 prior findings still unresolved — no new findingsThe previous Claude review (2026-06-27) identified three findings. All three are confirmed still present in the current HEAD after reviewing the commits since Unresolved Inline Findings1.
|
| Commit | Change | Verdict |
|---|---|---|
c6c4ed9 |
Full Stan model rewrite: LKJ + lognormal replaces inverse-Wishart; log1p in log_two_phase |
✓ Math correct (moment-matching derivation verified) |
7dde102 |
nBurnin ← nadapt; removed noisy cli_inform; seq_len(nchain); improved @returns |
✓ All prior P1 findings resolved |
9fa3c18 |
Added vignettes/articles/jags-to-stan.qmd |
✓ Correct |
e59d994 |
Removed unused copilot scaffolding | ✓ |
02af72b–3d7a25e |
Version bump, WORDLIST, pkgdown index | ✓ |
CLAUDE.md Requirements — All Met
| Requirement | Status |
|---|---|
dplyr::*_join() must set relationship |
✓ Both joins in run_mod_helpers.R use relationship = "many-to-one" |
seq_len() not 1:n |
✓ Fixed in 7dde102; initialize_prior_arrays also uses seq_len |
No .data in selection contexts |
✓ .data[[strat]] only inside dplyr::filter() (data-masking verb) |
tibble::as_tibble() not dplyr::as_tibble() |
✓ Correct in format_model_output() |
No base::merge() |
✓ |
| No deprecated lifecycle calls | ✓ |
| // output of prep_priors_stan(): mu_hyp, prec_hyp, omega, wishdf, | ||
| // prec_logy_hyp), so run_mod() (JAGS) and run_mod_stan() (Stan) fit the same | ||
| // model and their posteriors can be compared directly. I derive the HMC | ||
| // reparameterizations in the companion note model_stan_transform_from_jags.md. |
There was a problem hiding this comment.
Still unresolved from prior review — broken reference (hallucination risk): model_stan_transform_from_jags.md does not exist anywhere in the repository. Searching the full tree finds no such file. This reference was introduced in c6c4ed9 when the header was rewritten. The companion derivation was added simultaneously as vignettes/articles/jags-to-stan.qmd — update this line (and line 76 below) to point to that real document.
| // reparameterizations in the companion note model_stan_transform_from_jags.md. | |
| // reparameterizations in the companion vignette vignettes/articles/jags-to-stan.qmd. |
|
|
||
| transformed data { | ||
| // Moment-match the JAGS inverse-Wishart covariance prior to an LKJ + | ||
| // lognormal factorization (derivation in model_stan_transform_from_jags.md). |
There was a problem hiding this comment.
Still unresolved from prior review — same broken reference: model_stan_transform_from_jags.md does not exist. The derivation lives in vignettes/articles/jags-to-stan.qmd.
| // lognormal factorization (derivation in model_stan_transform_from_jags.md). | |
| // Moment-match the JAGS inverse-Wishart covariance prior to an LKJ + | |
| // lognormal factorization (derivation in vignettes/articles/jags-to-stan.qmd). |
| { | ||
| one_minus_shape <- 1 - shape_pop | ||
| # Compute the argument to log, ensuring it stays positive | ||
| log_arg <- y1_pop^one_minus_shape - | ||
| one_minus_shape * alpha_pop * (t - t1_pop) | ||
| # Use pmax to ensure log_arg > 0 (avoid NaN from log of negative) | ||
| (1 / one_minus_shape) * log(pmax(log_arg, .Machine$double.eps)) | ||
| } |
There was a problem hiding this comment.
Still unresolved from prior review — numerically unstable recovery formula: This implements the same expression the Stan model (log_two_phase) explicitly avoids. Near shape ≈ 1 (i.e., one_minus_shape → 0), log_arg → 1, so log(log_arg) → 0, and you get (1/0) * 0 → NaN. The pmax() guard protects against log(negative) only, not this 0/0 removable singularity.
Additionally, when pmax does clamp (e.g. large alpha*(t-t1)), dividing log(.Machine$double.eps) ≈ -36 by a small negative one_minus_shape gives a large positive result — the wrong sign.
The Stan model uses log1p to handle both issues cleanly (see functions {} block, line 53). Use the same formula here:
| { | |
| one_minus_shape <- 1 - shape_pop | |
| # Compute the argument to log, ensuring it stays positive | |
| log_arg <- y1_pop^one_minus_shape - | |
| one_minus_shape * alpha_pop * (t - t1_pop) | |
| # Use pmax to ensure log_arg > 0 (avoid NaN from log of negative) | |
| (1 / one_minus_shape) * log(pmax(log_arg, .Machine$double.eps)) | |
| } | |
| { | |
| # Numerically stable log1p form matching model.stan's log_two_phase(). | |
| # Stable as a = shape-1 -> 0 (removes the 0/0 singularity). | |
| a <- shape_pop - 1 # > 0 since shape_pop = exp(...) + 1 > 1 | |
| log(y1_pop) - | |
| log1p(a * alpha_pop * (t - t1_pop) * y1_pop^a) / a | |
| } |
|
|
||
| # Attach antigens and ids attributes for sample_predictive_stan() | ||
| attr(stan_out, "antigens") <- attr(longdata, "antigens") | ||
| attr(stan_out, "ids") <- attr(longdata, "ids") |
There was a problem hiding this comment.
Still unresolved from prior review — ids attribute reflects last stratum only: longdata here is always the result from the final loop iteration, so attr(stan_out, "ids") only contains that stratum's subject IDs. sample_predictive_stan() doesn't currently use this attribute, but any caller inspecting attr(result, "ids") after a stratified fit gets an incomplete and misleading list.
Options:
- Accumulate IDs inside the loop:
all_ids <- c(all_ids, attr(longdata, "ids"))and attachall_idshere. - Set to
NULLand document clearly that it is not populated for stratified models. - Document the limitation with an explicit comment so the next maintainer doesn't rely on it.
Convert JAGS Model to Stan ✅
This task successfully adds Stan support to the serodynamics package as an alternative to JAGS for Bayesian MCMC modeling. Both modeling backends can coexist, allowing users to choose their preferred framework.
Completed Work
Research Stan ecosystem and best practices
Create Stan model files
model.jagstoinst/extdata/model.stan(117 lines)model.dobson.jagstoinst/extdata/model.dobson.stan(19 lines)multi_normal_prec()for better performance and numerical stabilityCreate R infrastructure for Stan
prep_data_stan()function (75 lines) - converts data to Stan format with proper paddingprep_priors_stan()function (78 lines) - prepares Stan-compatible prior specificationsrun_mod_stan()function (197 lines) - complete Stan equivalent ofrun_mod()sample_predictive_stan()function - generates posterior predictive samples from fitted Stan modelsUpdate dependencies and documentation
Update README and documentation
Package validation
CI/Build improvements
*.knit.*pattern to.gitignoreto exclude intermediate knit filesREADME.knit.mdfrom git trackingcopilot-setup-steps.ymlworkflow\dontrun{}Test coverage
prep_priors_stan()(output structure, modifiability)prep_data_stan()(NA validation, data padding)run_mod_stan()(error handling, output structure, stratification support)skip_if_not_installed()for CI portabilityImplementation Details
Stan Model Conversion:
dnorm(mu, tau)tonormal(mu, sigma)wheresigma = 1/sqrt(tau)dwish(omega, df)toinv_wishart(df, omega_inv)whereomega_inv = inverse(omega)dgamma(a, b)parameters (same in both)multi_normal_prec()to accept precision matrices directly (better performance and numerical stability)Data Preparation:
prep_data_stan()handles thisnsmpl[subj]to ignore paddingadd_newpersonparameter inprep_data_stan()Posterior Predictive Sampling:
sample_predictive_stan()function for generating predictions from fitted Stan modelsCode Quality:
as.character()Integration:
sr_modelclass)CI Environment:
Example Code:
\dontrun{}for R CMD check compatibilityWhat Users Can Do Now
Use JAGS (existing functionality):
Use Stan (new functionality):
Both JAGS and Stan produce compatible outputs that work with the same analysis and visualization functions.
Note: Stan requires complete data (no NA values). For posterior predictive sampling with Stan models, use the
sample_predictive_stan()helper function which samples from the marginal posterior distribution of model parameters.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.