Skip to content

Add model definition vignette for estimate_dist()#1390

Open
seabbs-bot wants to merge 9 commits intomainfrom
issue-1388-estimate-dist-model-vignette
Open

Add model definition vignette for estimate_dist()#1390
seabbs-bot wants to merge 9 commits intomainfrom
issue-1388-estimate-dist-model-vignette

Conversation

@seabbs-bot
Copy link
Copy Markdown
Collaborator

@seabbs-bot seabbs-bot commented May 7, 2026

Description

This PR closes #1388.

Adds a model definition vignette for estimate_dist() matching the structure of the existing model definition vignettes for estimate_infections(), estimate_secondary(), and estimate_truncation().
While doing so the docs cluster around estimate_dist() was tightened up — duplicated material between the new model-def vignette and the existing fitting walkthrough was removed, the fitting walkthrough was renamed to a less collision-prone filename, and the pkgdown article structure was reorganised so the new model-def slots in correctly and the worked-example vignettes are no longer mis-categorised.

What changed

New model definition vignettevignettes/estimate_dist.Rmd. Reference-style prose plus maths (no code chunks, no .Rmd.orig needed). Covers the double interval censoring formulation, primary/secondary event window likelihood, right and left truncation, supported distributional families and parameterisations, default priors, the relationship to the vendored primarycensored Stan functions, and the model's limitations.

Fitting walkthrough renamedvignettes/estimate-dist.Rmdvignettes/estimate_dist_workflow.Rmd (and .orig, plus four figure PNGs and the matching CI workflow file). The hyphen vs underscore split between the two vignettes was too easy to confuse; the new name follows the existing <function>_workflow.Rmd convention used by estimate_infections_workflow.Rmd. The workflow vignette's introduction was also trimmed to remove duplication with the new model-def vignette and the conceptual delays.Rmd, with cross-links added in both directions.

pkgdown restructure_pkgdown.yml:

  • Added estimate_dist() to the navbar Models dropdown (was missing).
  • Articles index: estimate_dist (model def) added under "Model definitions"; estimate_dist_workflow moved out of "Model definitions" (it had been mis-categorised) into a new "Auxiliary models" group alongside forecasting_multiple_data_streams. The previously flat "Applied use" group is split into "Estimating Rt" (workflow, options, prior choice), "Auxiliary models", and "Production" (epinow). delays.Rmd stays in "Model definitions" as conceptual companion to the model defs.
  • Usage navbar dropdown gets matching subsection separators so the same grouping is visible in the menu.

model_overview.Rmd and model_features.Rmd — updated so:

  • Both link to the new model-def vignette.
  • model_overview.Rmd "Where to look next" mirrors the new pkgdown article structure (Estimating Rt / Auxiliary models / Production / Case studies) and includes the previously-missing estimate_dist_workflow and forecasting_multiple_data_streams entries.
  • model_features.Rmd "See also" links for estimate_dist features point to the model-def vignette (matching the convention used for the other estimation-model tables); the trailing "See also" also points at the worked example.
  • Two estimate_dist gaps in the features table closed: Weibull added to the Distribution families row, new "Untruncated approximation" row for obs_time_threshold.

Cross-references updatedR/estimate_dist.R, R/estimate_delay.R, man/estimate_dist.Rd, man/estimate_delay.Rd, and the .github/workflows/render-estimate_dist_workflow.yaml CI workflow.

NEWS.md — Documentation entry added for the new vignette.

Out of scope (follow-up work)

While auditing model_features.Rmd for completeness I noticed a few features unrelated to estimate_dist that aren't currently listed: obs_opts arguments dispersion, week_length, likelihood, return_likelihood; rt_opts arguments prior, use_rt, future, pop_period, pop_floor. Worth a separate issue rather than scope creep here.

Initial submission checklist

  • My PR is based on a package issue and I have explicitly linked it.
  • I have tested my changes locally (using devtools::test() and devtools::check()).
  • I have added or updated unit tests where necessary. N/A — documentation only.
  • I have updated the documentation if required and rebuilt docs if yes (using devtools::document()). New vignette renders cleanly; Rd files updated for the renamed cross-reference.
  • I have followed the established coding standards (and checked using lintr::lint_package()).
  • I have added a news item linked to this PR.

After the initial Pull Request

  • I have reviewed Checks for this PR and addressed any issues as far as I am able.

This was opened by a bot. Please ping @seabbs for any questions.

@seabbs-bot seabbs-bot requested a review from seabbs May 7, 2026 15:05
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5a40189f-66da-439d-85c7-b5c0de6d2ecf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

This PR adds a model-definition vignette for estimate_dist() alongside a NEWS.md entry. The vignette formally documents the interval-censored likelihood formulation, supported distributional families, prior specification, and implementation details via vendored Stan code from primarycensored.

Changes

estimate_dist() Model Definition Vignette

Layer / File(s) Summary
Announcement & Vignette Metadata
NEWS.md, vignettes/estimate_dist.Rmd
NEWS.md adds documentation entry for new model-definition vignette; vignette front matter specifies HTML output, table of contents, bibliography/citation configuration, and knitr options.
Model Overview & Notation
vignettes/estimate_dist.Rmd
Vignette introduces interval-censored primary/secondary event framework and formal notation for delay intervals, observation times, left-truncation points, and parametric delay distributions.
Likelihood Formulation
vignettes/estimate_dist.Rmd
Documents continuous likelihood via primary-censored CDF with right and optional left truncation, discrete interval observation likelihood as CDF differences, aggregated log-likelihood, and numerical approximation handling.
Supported Distributions & Priors
vignettes/estimate_dist.Rmd
Lists supported primary-event distributions (uniform, exponential growth), delay families (lognormal, gamma, normal, exp, weibull), parameterisations, analytical vs. numerical CDF availability, and prior specification with family-specific defaults.
Implementation & Limitations
vignettes/estimate_dist.Rmd
Describes relationship to upstream primarycensored package, Stan code vendoring/inclusion mechanism, distribution-code mapping, and explicit model limitations (supported families, left-truncation exposure, time-invariance assumption, measurement error, observation aggregation).

Possibly related issues

  • epiforecasts/EpiNow2#1388: Directly addresses the explicit request for a model-definition vignette for estimate_dist() parallel to existing vignettes for other estimate_*() functions.

Possibly related PRs

  • epiforecasts/EpiNow2#1317: This PR documents and references the new estimate_dist() functionality that was introduced in PR #1317.
  • epiforecasts/EpiNow2#1356: The main PR's NEWS.md and vignette document the estimate_dist()/primarycensored integration implemented by the retrieved PR.
  • epiforecasts/EpiNow2#1178: Both PRs update package documentation to reference {primarycensored} and related terms, touching similar documentation and references.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a model definition vignette for estimate_dist() function.
Linked Issues check ✅ Passed The pull request fulfils all primary objectives from issue #1388: creating a model-definition vignette covering double interval censoring, truncation handling, supported distributions, priors, and primarycensored relationships.
Out of Scope Changes check ✅ Passed The pull request includes a minor update to NEWS.md alongside the primary vignette addition; both changes are within scope and necessary for proper documentation of the new vignette.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-1388-estimate-dist-model-vignette

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

seabbs-bot added 8 commits May 7, 2026 16:10
Disambiguates from the new estimate_dist model-definition vignette and matches the existing <function>_workflow naming convention used elsewhere in the package.
Adds estimate_dist() to the Models navbar menu and the Model definitions article group, moves estimate_dist_workflow into a new Auxiliary models group with forecasting_multiple_data_streams, splits the flat Applied use group into Estimating Rt, Auxiliary models, and Production, and adds matching separators to the Usage navbar dropdown.
…oups

delays.Rmd is conceptual background, not a model definition. Moved to Start here in _pkgdown.yml and added a link to it from model_overview.Rmd. Updated the Where to look next section in model_overview.Rmd to mirror the new pkgdown article groups (Estimating Rt, Auxiliary models, Production, Case studies) and to include the previously-missing estimate_dist_workflow and forecasting_multiple_data_streams entries.
Revert the move to Start here. delays.Rmd stays alongside the model definitions and is referenced from model_overview.Rmd as conceptual background under the distribution model entry.
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.

Add a model definition vignette for estimate_dist()

1 participant