Repository navigation
Conversation
- Treat mean+SD (Eq 14/15) via a joint bivariate-normal likelihood on sample mean/variance using exact 3rd/4th central moments, replacing the independent-normal-theory approximation. - Treat reported order statistics, frequency-table counts, and double-censoring point observations as rounded values (resolution inferred per dataset) rather than exact continuous observations. - Fix a NaN-gradient/crash bug at delay=0 across all 5 families by special-casing F_D(0)=0 in the CDF functions, and revert the 0.1-day floor workaround this bug required across 16 real datasets.
- Treat mean+SD (Eq 14/15) via a joint bivariate-normal likelihood on sample mean/variance using exact 3rd/4th central moments, replacing the independent-normal-theory approximation. - Treat reported order statistics, frequency-table counts, and double-censoring point observations as rounded values (resolution inferred per dataset) rather than exact continuous observations. - Fix a NaN-gradient/crash bug at delay=0 across all 5 families by special-casing F_D(0)=0 in the CDF functions, and revert the 0.1-day floor workaround this bug required across 16 real datasets.
…t with the underlying data.
…l shape Cases 2/3 (only one side of exposure/event is a point) now get the same resolution-based rounding Case 1 already had. Case 1 itself is corrected: two independently day-rounded dates convolve to a triangular kernel of width 2*resolution, not a flat box of width resolution. All three now match Case 4's existing quadrature convention exactly.
…iciency levers Replace the shared scalar phi per pathogen with a non-centered hierarchical phi_d/omega reparameterization (both the primary and joint Stan models), so real between-study dispersion heterogeneity is no longer forced entirely into tau. The hierarchy is applied selectively based on a per-family identifiability analysis: lognormal, Weibull and Burr XII default to the hierarchical model; gamma reverts to a restored shared-scalar-phi variant, since its shape parameter's information about its own dispersion structurally vanishes at the large-shape end, unlike the other families; generalised gamma is eligible but gated behind the pre-existing should_attempt_gg() joint identifiability check. analysis/main_analysis.R now compiles and dispatches between the two model variants per family. Also fixes pre_inference_checks(), which still referenced the pre-point-4 parameter names and would hard-error against the new model. Three computational-efficiency changes: - n-adaptive quadrature panel count for the order-statistic rounding likelihood: the fixed 24-panel scheme was calibrated for the corpus’s largest dataset and is overkill for most of it. Panel count is now computed per dataset from a method-of-moments dispersion guess (fully static, no change to Stan's autodiff model), with family-specific safety margins and gamma excluded (its own worst-case margin would require an intractable node count). Validated to agree with the fixed-panel scheme to 3e-11 in log-probability at a fixed parameter point, with a measured 6.92x speedup on a real fit. - fit_with_escalation(): tries increasingly conservative MCMC settings, stopping once Rhat<1.05 and divergence rate <1%, falling back to the original production settings (adapt_delta=0.999) verbatim as the final tier rather than relaxing them, since those were originally needed for some chains to converge at all. - fit_corpus_parallel(): parallelizes fitting across pathogen/family combinations, reserving cores (default 4 of 12) so the machine stays usable during a run analysis/checkpoint_run_point4.R is a reduced-settings script for validating all of the above across a representative pathogen subset before a full run.
Extend more fully across distributions
There was a problem hiding this comment.
Pull request overview
This PR updates the ddsynth modeling pipeline to improve likelihood correctness and stability across five distribution families, with a particular focus on (a) handling day-rounded observations consistently and (b) improving the mean+SD likelihood using exact finite-sample moments. It also restructures corpus fitting to run tasks in parallel with resumability and adds several new utility and diagnostic functions.
Changes:
- Updates Stan likelihoods to (1) special-case CDF behavior at delay=0 to avoid NaN gradients, (2) treat order statistics / frequency-table point values / point observations in double-censoring as rounded values, and (3) use a joint bivariate-normal likelihood for (sample mean, sample variance) using exact 3rd/4th central moments.
- Introduces per-study dispersion hierarchy (
phi_d) with new hyperparameteromega, updates init/tests, and expands exports/documentation for new utilities (resolution detection, safe-family gating, parallel fitting, and temporal diagnostics). - Refactors
analysis/main_analysis.Rto queue all (pathogen, analysis, family) fits into a bounded parallel executor with per-task persistence and resumability.
Reviewed changes
Copilot reviewed 11 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/testthat/test-utils.R | Updates init-name expectations for the new Stan parameterization (log_phi0, log_omega, log_phi_d_raw). |
| R/zzz.R | Adds new globalVariables entries for additional outlier flags. |
| R/ddsynth_datasets.R | Reverts multiple datasets’ 0.1-day “floor” back to true zeros now that delay=0 is handled safely. |
| NAMESPACE | Exports newly added helpers for fitting, gating, resolution detection, and plotting. |
| man/prepare_stan_data_from_datasets.Rd | Documents new double-censoring summary types and new prior knobs (log_omega_*), plus updated backward-compat notes. |
| man/pre_inference_checks.Rd | Documents expanded dist types and a new spread-based outlier criterion. |
| man/plot_temporal_trend.Rd | Adds documentation for a new type-7 temporal diagnostic plot. |
| man/plot_main_figure.Rd | Fixes ggplot2 link target. |
| man/merge_parallel_fit_results.Rd | Adds documentation for merging per-task results from parallel corpus runs. |
| man/generate_hierarchical_data_mixed.Rd | Adds simulation options for rounding order stats and recording resolution. |
| man/gamma_phi_extreme_safe.Rd | Adds documentation for gamma “extreme implied shape” gating. |
| man/fit_with_escalation.Rd | Adds documentation for tiered sampling escalation until Rhat/divergence criteria are met. |
| man/fit_corpus_parallel.Rd | Adds documentation for bounded-concurrency parallel corpus fitting with timeouts/callback. |
| man/extract_posterior_draws.Rd | Adds documentation for extracting parameter draws for downstream diagnostics. |
| man/detect_resolution.Rd | Adds documentation for inferring rounding resolution from reported stats. |
| man/ddsynth-package.Rd | Adds package logo figure and author listing. |
| man/compute_predictive_cdf.Rd | Updates rstan link target. |
| man/burr_phid_reliable.Rd | Adds documentation for Burr XII hierarchy reliability gating. |
| inst/stan/hierarchical_data_synthesis_summary_stats.stan | Major likelihood + parameterization changes (rounding, mean/variance BVN likelihood, phi_d hierarchy, delay=0 guard). |
| inst/stan/hierarchical_data_synthesis_summary_stats_joint.stan | Mirrors the above in the “joint” order-statistic model variant. |
| inst/stan/hierarchical_data_synthesis_summary_stats_joint_shared_phi.stan | Adds shared-phi joint variant (e.g., for gamma) while retaining newer rounding + delay=0 protections. |
| DESCRIPTION | Switches to Config/roxygen2/version (8.0.0) and removes RoxygenNote. |
| analysis/main_analysis.R | Refactors main analysis into queued tasks executed via bounded parallel runner with resumability and per-task artifacts. |
Files not reviewed (14)
- man/burr_phid_reliable.Rd: Generated file
- man/compute_predictive_cdf.Rd: Generated file
- man/ddsynth-package.Rd: Generated file
- man/detect_resolution.Rd: Generated file
- man/extract_posterior_draws.Rd: Generated file
- man/fit_corpus_parallel.Rd: Generated file
- man/fit_with_escalation.Rd: Generated file
- man/gamma_phi_extreme_safe.Rd: Generated file
- man/generate_hierarchical_data_mixed.Rd: Generated file
- man/merge_parallel_fit_results.Rd: Generated file
- man/plot_main_figure.Rd: Generated file
- man/plot_temporal_trend.Rd: Generated file
- man/pre_inference_checks.Rd: Generated file
- man/prepare_stan_data_from_datasets.Rd: Generated file
Suppressed comments (1)
inst/stan/hierarchical_data_synthesis_summary_stats.stan:830
generated quantitiescurrently computespred_*usingphi = phi0only, ignoring the new between-study dispersion heterogeneity (omega) and the per-studyphi_dparameterisation. This makespred_median,pred_q95, etc. inconsistent with the hierarchical model and can bias the reported predictive summaries (these outputs are used downstream in the analysis scripts/tables).
// TODO(point 4, task 17): phi is currently evaluated at the population
// mean phi0 only, i.e. these predictive quantities do NOT yet marginalize
// over between-study dispersion heterogeneity (omega), unlike location
// (which already does via loc_sample below). This is a placeholder to
// keep the model compiling after the phi -> phi_d reparameterization;
// fixing it properly (sampling phi alongside loc per Monte Carlo draw,
// mirroring locs <- rnorm(...)) is tracked as its own task since it
// changes Table 1's reported P95/median, not just machinery.
{
real phi = phi0;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 25 changed files in this pull request and generated 3 comments.
Files not reviewed (14)
- man/burr_phid_reliable.Rd: Generated file
- man/compute_predictive_cdf.Rd: Generated file
- man/ddsynth-package.Rd: Generated file
- man/detect_resolution.Rd: Generated file
- man/extract_posterior_draws.Rd: Generated file
- man/fit_corpus_parallel.Rd: Generated file
- man/fit_with_escalation.Rd: Generated file
- man/gamma_phi_extreme_safe.Rd: Generated file
- man/generate_hierarchical_data_mixed.Rd: Generated file
- man/merge_parallel_fit_results.Rd: Generated file
- man/plot_main_figure.Rd: Generated file
- man/plot_temporal_trend.Rd: Generated file
- man/pre_inference_checks.Rd: Generated file
- man/prepare_stan_data_from_datasets.Rd: Generated file
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 25 changed files in this pull request and generated no new comments.
Files not reviewed (14)
- man/burr_phid_reliable.Rd: Generated file
- man/compute_predictive_cdf.Rd: Generated file
- man/ddsynth-package.Rd: Generated file
- man/detect_resolution.Rd: Generated file
- man/extract_posterior_draws.Rd: Generated file
- man/fit_corpus_parallel.Rd: Generated file
- man/fit_with_escalation.Rd: Generated file
- man/gamma_phi_extreme_safe.Rd: Generated file
- man/generate_hierarchical_data_mixed.Rd: Generated file
- man/merge_parallel_fit_results.Rd: Generated file
- man/plot_main_figure.Rd: Generated file
- man/plot_temporal_trend.Rd: Generated file
- man/pre_inference_checks.Rd: Generated file
- man/prepare_stan_data_from_datasets.Rd: Generated file
Suppressed comments (3)
man/prepare_stan_data_from_datasets.Rd:78
- The \value section says the returned Stan data list always includes only
freq_lower/freq_upper, but the Note below states the Stan model also requiresevent_lowerandevent_upperto always be present. This is a documentation inconsistency for callers constructing data manually.
inst/stan/hierarchical_data_synthesis_summary_stats.stan:836 pred_*generated quantities are still computed usingphi = phi0only (see TODO), so posterior predictive summaries ignore dispersion heterogeneity (omega) even whenn_datasets >= 5and location heterogeneity (tau) is already being marginalised vialoc_sample. This makespred_*internally inconsistent with the new per-studyphi_dparameterisation and can materially change reported predictive medians/quantiles (as noted in the TODO), which risks incorrect downstream summaries/figures that rely on these fields.
// TODO(point 4, task 17): phi is currently evaluated at the population
// mean phi0 only, i.e. these predictive quantities do NOT yet marginalize
// over between-study dispersion heterogeneity (omega), unlike location
// (which already does via loc_sample below). This is a placeholder to
// keep the model compiling after the phi -> phi_d reparameterization;
// fixing it properly (sampling phi alongside loc per Monte Carlo draw,
// mirroring locs <- rnorm(...)) is tracked as its own task since it
// changes Table 1's reported P95/median, not just machinery.
{
real phi = phi0;
if (n_datasets < 5) {
inst/stan/hierarchical_data_synthesis_summary_stats_joint.stan:771
pred_*generated quantities are still computed usingphi = phi0only (see TODO), so posterior predictive summaries ignore dispersion heterogeneity (omega) even whenn_datasets >= 5and location heterogeneity (tau) is being marginalised. With the new hierarchicalphi_dparameterisation, this can make reported predictive quantiles inconsistent with the fitted model and any downstream code that consumespred_*.
// TODO(point 4, task 17): phi is currently evaluated at the population
// mean phi0 only; these predictive quantities do not yet marginalize over
// between-study dispersion heterogeneity (omega). See the matching TODO
// in hierarchical_data_synthesis_summary_stats.stan.
{
real phi = phi0;
if (n_datasets < 5) {
real loc_pred = mean(loc_d);
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fix likelihood correctness: rounding, zero-values, mean/SD moments
Treat mean+SD (Eq 14/15) via a joint bivariate-normal likelihood on sample mean/variance using exact 3rd/4th central moments, replacing the independent-normal-theory approximation.
Treat reported order statistics, frequency-table counts, and double-censoring point observations as rounded values (resolution inferred per dataset) rather than exact continuous observations.
Fix a NaN-gradient/crash bug at delay=0 across all 5 families by special-casing F_D(0)=0 in the CDF functions, and revert the 0.1-day floor workaround this bug required across 16 real datasets.