Apply the multiplicative deviation to top and bot - #295
Conversation
Extends #257's parameterisation to pgl() and (par | group). A deviation on top or bot is applied multiplicatively, so no leapfrog step can take the parameter outside the support of the mean. The parameter keeps its name, prior and meaning; the generated deviation (botgl) and intermediate (bnecbot) are what is new. adapt_delta is now raised only where the equation's mean can leave the support with every parameter inside it. Closes #294 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rrg3VivUSguUgzCQStFC6j
beckyfisher
left a comment
There was a problem hiding this comment.
Review of issue-294-par-transform at 62fe1ee1, against origin/dev. The full diff was read hunk by hunk, and formula construction, prior generation and the adapt_delta gate were exercised directly in R 4.6.1 with brms 2.23.0 in a temporary worktree.
The change is structurally sound on the points that were checked. The substitution into the curve is symbol-based and composes correctly with ogl(), with a combined (top + bot | grp) term, with a model that has no bot, and with nechormepwr01's 1/top. brms::make_stancode() succeeds for Beta, Gamma, poisson, negbinomial, the zero_inflated_* families and hurdle_gamma. The default top and bot priors are bounded to the family's support for every family the transform admits, including the zero-inflated ones. All four group_inits() call sites were updated for the signature change.
Six findings follow. Five are inline. The sixth is recorded here because R/ecx.R has no diff hunk to anchor it to.
R/ecx.R:407 --- type = "relative" uses a denominator that is not the asymptote of its numerator
ecx_asymptote() reads b_bot_Intercept as the asymptote of the curve returned by posterior_epred(re_formula = NA). Under the transform that curve declines towards bnecbot = bot * exp(b_botgl_Intercept) / (1 - bot + bot * exp(b_botgl_Intercept)). re_formula = NA sets r_grp__botgl to zero but not the population intercept b_botgl_Intercept, which is unidentified against bot --- as the comment added at R/define_prior.R:672 states --- and is constrained only by its normal(0, s) prior, with s up to 1 under the cap.
For bnec(y ~ crf(x, "nec4param") + (bot | site), family = "beta"), ecx(fit, type = "relative") and ecnsec() therefore divide by a quantity the curve does not decline towards. The same applies to the top and bot estimates expand_nec() extracts at R/expand_classes.R:29 and summary() reports: they are the population-level non-linear parameters, not the asymptotes of the population-level curve. The PR body's statement that b_bot_Intercept and ecx() are unaffected holds for the families the transform does not apply to.
Verified by reading the generated Stan code for resp ~ crf(pred, "nec4param") + (bot | grp) under validate_family("Beta") on this branch, which declares b_bot and b_botgl as separate population-level vectors, and by re_formula's documented behaviour in brms 2.23.0. Not verified by a fitted model.
Not verified
The updated test files were not run to completion. NOT_CRAN=true testthat::test_file() over test-mu_support.R, test-wrangle_model_formula.R, test-define_prior.R, test-check_priors.R, test-check_data.R and test-inits_functions.R completed test-mu_support.R with no failures reported and was still inside test-wrangle_model_formula.R after approximately four minutes, at which point it was terminated. No pass or fail is reported for the other five files. The new skip_on_cran() sweep in test-wrangle_model_formula.R builds 207 brms::make_stancode() calls (23 equations x 3 families x 3 terms) and accounts for that runtime.
- drop the deviation's population intercept: botgl ~ 0 + (1 | group). It was exactly unidentified against bot, so b_bot_Intercept was not the asymptote ecx(type = "relative") and summary() report. - route B of disp() reads the curve as built rather than the bf_ template, which discarded the transform. - decide the adapt_delta raise on mu_confined_by_pars(), so the hormesis equations keep it on positive-support families. - refuse a user prior that leaves top or bot unbounded under a transformed term. - test for the grouping term, not the variable name, so a second grouping whose name is a substring of the first is not dropped. - cap the converted prior scale at 1 / narrow, so prior_type is not inert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rrg3VivUSguUgzCQStFC6j
|
All six findings were verified against the branch and all six were real. Fixed at
|
|
The body has been updated to describe the branch at The parameterisation sketch and the The
The ungrouped arm reproduces to the digit, so the differences in the grouped This also closes the Two things not previously recorded. Also corrected in the body, all consequences of the review fixes rather than
🤖 Generated with Claude Code |
What
A group-level deviation on
toporbotis now applied multiplicativelyrather than added to the parameter, wherever the likelihood constrains the
mean. This is #257's parameterisation one level down, and it covers
pgl()and an explicit
(par | group)term.botis the parameter it matters for: it is the lower asymptote, it isroutinely estimated close to zero, and the deviation
brmsadded to it wasunconstrained.
Why it matters
The
adapt_delta = 0.99mitigation was not reliable. #294 measured it removingthe divergences on one dataset, reducing them twelve-fold on a second, and
failing on a third:
pgl()at 0.99 gave 1900 divergent transitions of 2000 witha maximum R-hat of 2.63. That is the signature PR #228's grouping vignette
investigation reported and could not account for.
topandbotkeep their names, their priors and their meanings, sob_bot_Intercept,ecx(),nsec()and the model-averaging machinery areunaffected. What a user sees change is that a term on
botis nowsummarised as
sd(botgl_Intercept)rather thansd(bot_Intercept), and is onthe log-odds or log scale rather than on the response scale. A prior the user
supplies on
toporbotthat does not bound the parameter to the family'ssupport is now refused where a group-level term on it would be transformed,
because the multiplicative form is only defined inside that range.
Evidence
All arms below: R 4.6.1, brms 2.23.0, rstan 2.32.7, two chains,
iter = 2000,warmup = 1000,seed = 228, at Stan's defaultadapt_delta = 0.8. Thecomparison column is #294's measurement of
devat20053c95.The
herbicidetable is measured at6f43cca0, the current head. The twolocal-data tables below it were measured at
62fe1ee1, before the deviation'spopulation intercept was removed. That change alters the parameter set the
sampler explores, so those two tables describe the earlier parameterisation.
They are retained because the arm they were run for --- the one on which the
adapt_deltamitigation failed outright ondev--- is the strongest evidencein this pull request. Both use git-ignored local data. Each was fitted with
nec4param:y ~ crf(log_x, "nec4param")onexample_pgl.csv, andsuc | trials(tot) ~ crf(conc, "nec4param")withfamily = "beta_binomial"and
conc = as.numeric(Measured.Conc)onexample_dat.csv, both atseed = 228, two chains and 2000 iterations. Re-measurement at6f43cca0isoutstanding.
herbicide,Beta(link = "identity"),nec4paramonlog(concentration).dev(bot | herbicide)(top | herbicide)pgl(herbicide)The ungrouped arm reproduces the figures measured at
62fe1ee1to the digit,so the two grouped arms differing from them is attributable to the
parameterisation rather than to run-to-run variation.
pgl(herbicide)reported27 transitions exceeding the maximum treedepth, which is a sampling efficiency
warning rather than an excursion and is not what this change addresses.
ignore/example_dat.csv, 109 rows, five groups,beta_binomial,suc | trials(tot)--- the arm on which the mitigation failed outright. Bothfiles are git-ignored local data, so these are reported for the pattern rather
than as a reproduction.
devpgl(test)(nec | test)(bot | test)ignore/example_pgl.csv, 176 rows, four groups,Beta.dev(bot | plate)(top | plate)pgl(plate)The
necarms are the control: they were already clean at Stan's default andare unchanged, which is why they are not transformed.
Formula construction, prior generation and Stan code generation were swept over
all 23 equations x 4 term types x 3 families (
Beta,Gamma,gaussian): nofailures. That sweep is now a
skip_on_cran()test.Implementation detail
The parameterisation
b_bot_Interceptis read byecx.Rand by the summary machinery, so theparameter could not be renamed. The generated terms are renamed instead,
following
bnecmufrom #257:botstays a population-level non-linear parameter. The deviation iszero-centred and
m * exp(0) == m, so a fit with no group-level effect is thefit
devproduces. On (0, Inf) the form isbot * exp(botgl). Neither iswritten as the literal
inv_logit(logit(m) + o)sandwich, for the reason #257records:
logit(m)underflows to-Infasm -> 0, which is exactly wherebotlives.The deviation has no population intercept, which is what keeps
botinterpretable.
bnecbotdepends onbotandbotglonly through theircombination, so a free
b_botglwould be exactly unidentified againstbot:the two trade off along a ridge with no change to the likelihood, and
b_bot_Interceptwould no longer be the asymptote the population-level curvedeclines towards --- which
ecx(type = "relative")divides by atR/ecx.R:407andexpand_nec()reports. Writing the sub-formula with0 +leaves exactly the parameters the additive form had:
bot, the group-levelstandard deviation, and the deviations themselves, which the hierarchical prior
centres on zero. Confirmed on a fitted
(bot | herbicide)model, whoseparameter set is
b_bot_Intercept,b_top_Intercept,b_beta_Intercept,b_nec_Interceptandsd_herbicide__botgl_Intercept, with nob_botgl.ogl()keeps its intercept and the zero-centred prior #257 gave it, because itis documented as adding a population-level parameter of its own.
Checked against brms 2.23.0 that the sub-formulas may be given in any order ---
brms resolves them by name and the generated Stan code is identical --- so
#257's
bnecmuprepend and the appended parameter terms compose without anordering rule.
Three rulings, taken from the issue's "stop and ask"
Only
topandbotare transformed.necandec50are on the predictorscale and are routinely negative on a log predictor, so
log/logitof them isundefined;
beta,slope,dandfare dimensionless and enter through anexponential. None is bounded by the likelihood. This also makes
pgl(g)expandto exactly what writing the equivalent
(par | group)terms by hand gives,which is what
pgl()is documented to mean. A test asserts the two areidentical.
The gate is the family, not the equation.
ogl_transform_kind()needs themean provably strictly inside its support and so refuses the hormesis
equations. A parameter is not the mean:
define_prior()boundstopandbotto the family's support with
lb/ubwhatever equation they appear in, somodel_mu_ranges()is not consulted and the transform is defined fornechorme,nechorme4,nechormepwr01,ecxhormebc4andecxhormebc5aswell. Those equations keep the raised
adapt_deltafor the separate reasonbelow.
That bound is what makes the transform safe, and a user-supplied prior can
remove it:
fill_missing_priors()keeps a user row and fills only what isabsent, so a
normal(0.2, 0.5)onbotmerges withlbandubbothNA.Outside
[0, 1]the collapsed expression has a pole and returns a large finitenumber, which Stan accepts, where the additive form it replaces produced an
out-of-range mean that Stan rejected visibly.
check_transformed_par_bounds()refuses such a prior after the merge, naming the parameter and the bounds to
add, and fires only where the term would be transformed.
The prior is #257's delta-method conversion at
mean(y), capped in bothbranches. #257 caps the
logbranch only, on the argument that thelogitratio is self-limiting at the response mean; that argument does not hold for a
parameter sitting near zero. The cap is
1 / narrowrather than 1, becausenarrowis applied tos_ybefore the conversion and so the cap belongs onthe same scale: capping at 1 last made
regularizinganduninformativereturn the same prior wherever the uninformative converted width already
reached the cap. The
oglconversion is deliberately left as #257 set it, sono landed fit changes.
The
adapt_deltaruleonly_oglgeneralises to one condition. With every deviation applied on a scaleit cannot leave, the question is whether any group-level term can still put
muoutside the support. It cannot, provided the mean is confined by its own
parameters:
topandbotstay inside the support, and for an equation whosemean lies between
botandtopa deviation onnec,beta,slope,dorfleavesmubetween two in-support values.mu_confined_by_pars()answersthat question, testing
below_zero,can_exceed_oneandceiling_at_onewhatever the support is:
The gate is not delegated to
ogl_transform_kind(). That function testscan_exceed_oneon the(0, 1)branch only, because there the question iswhether a
logitis defined; the raise turns on a different question. Anoglterm is the one case where it still decides, because
oglis not transformedat all where it refuses.
The raise is kept for
neclin,neclinhormeandecxlin, which are unboundedbelow, and for the six hormesis equations, on every family: an excess term
in
exp(slope) * xmakes their mean negative for a sufficiently negativepredictor, and
crf(log(concentration), ...)supplies one as a matter ofcourse. The "permanently" comment at
R/helpers.R:773is corrected.This is a wider narrowing than #294's own wording implies. It was taken because
the definition of done requires a
pgl()fit to sample at Stan's default, whichis only reachable if the raise is dropped for a term set that includes
untransformed
necandbetaterms.Files
R/mu_support.R---par_transform_kind(),par_transform_pars(),par_is_transformed(),par_gl_names(),mu_confined_by_pars(),generated_term_names(),group_zero_intercepts().R/bayesnecformula.R---add_par_gl_term(); both branches ofadd_formula_glef(), which now read the model's parameter set once beforeeither runs so that a generated term cannot be mistaken for a parameter;
parse_group_terms()documentation, which keeps reporting the structure theuser wrote because it is not given a family;
has_gl_term(), which tests forthe term a grouping would add rather than for the variable's name anywhere in
the deparsed right-hand side, in both the transformed and the untransformed
path; and a
?bayesnecformulasection on the scale a deviation is appliedon.
R/define_prior.R---converted_scale(), capped at1 / narrowso thatprior_typeis not inert where the uninformative converted width alreadyexceeds the cap, and the nlpar the group-level standard deviation is declared
under, which is the deviation term's name rather than the parameter's. A
transformed deviation is given no intercept prior, because it has no
intercept.
R/disp_model.R--- route B ofdisp()reads the curve as assembled ratherthan rebuilding it from the
bf_<model>template, which discarded thesubstitution and left
phia function of the population-levelbot. Route Bnow also sees the
ogl()deviation, which it never had.R/helpers.R--- theadapt_deltarule,check_transformed_par_bounds(),which refuses a user prior leaving
toporbotunbounded under atransformed term, and the init-prior filter.
R/inits_functions.R,R/fit_bayesnec.R---group_inits()takesgroup_specin place of theoglflag and readsgroup_zero_intercepts(),which returns
oglalone: a transformed deviation has no intercept toinitialise.
R/check_data.R---topgl,botgl,bnectopandbnecbotjoinbnecmuand
oglas refused data column names. The set is refused in full whateverthe formula is, so the same data is not accepted on one call and refused on
the next.
NEWS.md,vignettes/example3.Rmd.orig.vignettes/example3.Rmdis not regenerated here; the precompile runs once forthe whole package (see #190).
Existing tests updated rather than deleted
Four asserted behaviour this change alters, and each was rebased onto what it
was actually testing:
test-define_prior.R, hurdle group term: the grouping now lands ontopgl,and what the test asserts --- that it reaches the mu block and not the hu
block --- is unchanged.
test-define_prior.R,prior_typeround trip: the width is now the convertedone, and
narrowstill enters before the conversion, so regularizing is stillhalf uninformative.
test-inits_functions.R, theadapt_deltagate: rebased ontoneclinso itstill tests the family gate rather than the equation gate that now decides
nec3param, with the new behaviour asserted alongside.test-check_priors.R: "a pgl term is not transformed in this landing" isreplaced by the rule that now holds, with both sides of it asserted.
Four
group_inits()call sites were updated for the signature change.Definition of done
(bot | group)andpgl()on a constrained family sample withoutexcursion-driven divergences at Stan's default
adapt_delta.top,bot,necandbetakeep their meanings.Updated after review (
6f43cca0). Six findings, all real, all fixed, eachwith a reply on its own thread. The sections above describe the design as it
now stands rather than as it was reviewed. The substantive finding was that the
deviation was written
botgl ~ 1 + (1 | group), giving it a populationintercept exactly unidentified against
bot; it is now written with0 +andhas none. The other five: route B of
disp()read thebf_template ratherthan the assembled curve; the
adapt_deltagate is decided bymu_confined_by_pars(), so the hormesis equations keep the raise on everyfamily; a user prior leaving
toporbotunbounded under a transformed termis refused; the duplicate-grouping check tests for the term rather than the
variable name; and the converted prior scale is capped at
1 / narrow, soprior_typeis not inert. Full suite: 0 failures, andR-CMD-checkis greenon ubuntu release and devel, macOS and Windows.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Rrg3VivUSguUgzCQStFC6j