Put the normal family's default priors on the outcome scale - #115
Merged
Merged
Conversation
For family = "normal" the intercepts and the identity-link coefficients are in the outcome's own units, and so is the residual SD under either link. The defaults were fixed: normal(0, 10) on the intercepts, normal(0, 2.5) on the coefficients, and a half-normal with scale 2.5 on sigma. On an outcome such as a 0 to 100 score or a blood pressure in mmHg they are informative. The intercept prior pulls each intercept toward zero, the comparator's (which rests on one aggregate mean) the most, so the mean difference moves; the sigma prior understates the residual SD and narrows every interval. On the bundled shoulder example the defaults gave a mean difference of -6.84 (posterior SD 2.90) against -7.75 (3.13) with vague priors and -7.72 from stc(), with sigma 19.2 against 23.0. On a simulated blood pressure outcome with a 60-patient comparator the defaults moved the estimate from -1.35 to +2.91, two posterior SDs, which stc() also puts at -1.35. With sd(y) the IPD outcome SD, a package default is now read in units of sd(y) wherever the parameter is in outcome units: normal(0, 10 * sd(y)) for the intercepts and normal(0, 2.5 * sd(y)) for the coefficients under the identity link, and a half-normal with scale 2.5 * sd(y) for sigma under either link. Under the identity link autoscale = TRUE also multiplies a coefficient's scale by sd(y), which is what makes it unit-free there. Priors written out in full are used as given, and nothing changes for the other families. The fit keeps the priors as passed, so prior_sensitivity() replays them unchanged, and records the scales used: prior_summary() prints them and plot_prior_posterior() draws them. A swept default prior_beta keeps its outcome-SD units, so the sweep row at the original scale reproduces the fit. The continuous-outcomes vignette is re-knitted: the shoulder mean difference is now -7.67 (SD 3.09) and the coefficients match least squares. The fitting-and-diagnostics vignette gains one sentence and is re-rendered. Checks: pure-R suite 0 failures, 0 errors, 92 skips, 2,855 passes; the new test file and the Stan-enabled tests in the nine files that fit normal models or read priors pass; the new blocks fail against main except the one pinning that user priors pass through unchanged. Version stays 0.1.0.9000.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: choxos/mlumr/.coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (13)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughNormal-family default and autoscaled priors now use the IPD outcome SD. Resolved scales flow through fitting, metadata, summaries, plots, sensitivity analysis, tests, and documentation. Explicit priors remain unchanged. ChangesNormal prior scaling
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Fit
participant PriorBuilder
participant Metadata
participant Reporting
Fit->>PriorBuilder: Compute outcome-scaled priors using sd_y
PriorBuilder-->>Fit: Return resolved prior fields and scale metadata
Fit->>Metadata: Store resolved intercept, beta, sigma, and sd_y values
Metadata->>Reporting: Supply resolved priors for summaries and overlays
Merge Risk: ⚪ Minimal · up to Normal-outcome priors and their reporting are consistently updated to use outcome-scale units, with explicit priors preserved. The change is ready to merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
choxos
added a commit
that referenced
this pull request
Sep 24, 2026
The lesson loaded mlumr's R code, and described its behavior, at commit 4cfd366 (September 12). Four things it taught changed on main since then: * check_identification() no longer reports target_in_span, target_in_declared_span or target_span_gap (#101). Chapter 11's narration and the package notes described that target check. * predict(), marginal_effects() and the conditional summaries no longer carry n_draws and n_draws_used; they warn once when they leave out NA or NaN draws (#104). Chapter 12's narration, the Incomplete summary card, the report checklist and the capstone asked for those counts. * The checks on tied reconstructed event times that refused or warned before sampling are gone (#100). The survival distributions panel and the notes still named them. * For a normal outcome with an identity link, autoscale = TRUE and the default priors carry the IPD outcome SD (#115). The priors panel and the notes said only that autoscale divides by the covariate SD. What changes: * lesson.sh pins mlumr 965dfc5; README, sources.html and the package notes link that commit. The binomial Stan programs are identical, so the WebAssembly models stay. * Two narration sentences (chapters 11 and 12) and the panel, card, checklist and capstone text above are corrected. The survival panel and the notes now say that any at_time other than 0 is refused under a shared baseline, as the package does. * sources.html adds the published ISPOR Europe 2025 abstract next to the preprint, as the package README does. * workflow.R passes prior_beta_comparator to the relaxed model only; the SPFA refits used to receive it and print the package's warning twice. * scenes/native-record.json is rerun at 965dfc5. Every fitted number is unchanged to the last recorded digit; only the commit, the compiled code's hash, the script hash and the run time differ. * dist/ rebuilt: narration 1686.81 s, only the three changed caption lines differ. Checks: lesson.sh test (74 unit tests), the adapter against a native checkout at 965dfc5 (22 checks, Stan data equal to the package's), dist-manifest verify, and browser-qa.mjs with the R and Stan runtimes (0 errors, both browser fits). Every function and named argument the lesson uses exists at 965dfc5. QA.md and package-notes.md record the runs. Review fix: set_agd_surv() still validates the reconstructed times, so the survival panel and the notes say that only the tie screening before sampling is gone, and the QA record's commands use a quoted path variable.
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.
What was wrong
For
family = "normal"the intercepts and the identity-link coefficients are in the outcome's own units, and so is the residual SD under either link. The package defaults were fixed scales:normal(0, 10)on the intercepts,normal(0, 2.5)on the coefficients, and a half-normal with scale 2.5 onsigma. For an outcome such as a 0 to 100 score or a blood pressure in mmHg these are informative:se^2 / (10^2 + se^2)of its value. The comparator intercept rests on one aggregate mean, usually with the larger standard error, so it is pulled the most and the mean difference moves.sigmaprior pulls the residual SD down, which overstates the precision of the IPD and narrows every interval.autoscale = TRUEdivided a coefficient's scale bysd(x)but left the outcome's units in it, so it was not unit-free for this family.prior_sensitivity()sweepsprior_betaonly, so it could not show any of this.stc()What changes
With
sd(y)the IPD outcome SD:sd(y)wherever the parameter is in outcome units:normal(0, 10 * sd(y))for the intercepts andnormal(0, 2.5 * sd(y))for the coefficients under the identity link, and a half-normal with scale2.5 * sd(y)forsigmaunder either link.autoscale = TRUEmultiplies a coefficient's scale bysd(y)as well as dividing it bysd(x).prior_sensitivity()replays them unchanged, and records the scales used:prior_summary()prints them andplot_prior_posterior()draws them. A swept defaultprior_betakeeps its outcome-SD units, so the sweep row at the original scale reproduces the fit.prior_normal(),default_priors,mlumr()andprior_sensitivity()updated.Vignettes
continuous-outcomesre-knitted: the shoulder mean difference is now -7.67 (SD 3.09),sigma22.9, and the coefficients match least squares. The prior paragraphs describe the outcome-scaled defaults. Every interpretive sentence was checked against the new output and still holds.fitting-and-diagnosticsgains one sentence on the normal family, prose only, re-rendered without Stan. Its HTML also picks up the duplicatelibrary(mlumr)line that 977eeb4 removed from the source but not from the rendered page.subgroup-identificationreads stored simulation results and is not rerun. Its normal arm has outcome SD near 1, so the new defaults are about 10% wider than the ones it used; the stored results stay a faithful record of the run that produced them.Checks
tests/testthat/test-normal-prior-scale.R: resolved Stan fields for identity and log links, user priors, autoscale, the recorded metadata and its printout, the overlay prior, the sensitivity sweep units, and one Stan-enabled fit that must matchstc()within 0.5 (it was 4.3 away before).main, every new block fails except the one pinning that user priors pass through unchanged, which is a deliberate no-change guard.test-mlumr,test-mlumr-validation,test-normal-residual-variation,test-plot,test-prior_sensitivity,test-relaxed-prior-comparator,test-regressions,test-effect-scale-and-validation,test-identification): all pass.tests/repo: everything passes except the local tarball build, which ran out of disk copying untracked files; CI builds from a clean checkout.Version stays
0.1.0.9000.Summary by CodeRabbit
New Features
Documentation
Important