Add a vignette on grouping and factor covariates (#6, #33) - #228
beckyfisher wants to merge 26 commits into
Conversation
Review — phase 2 of
|
| required | where |
|---|---|
| all three group-level term types, and when each is right | ## ogl(), ## pgl(), ## (par | group) |
| what the terms do to the curve and to the estimates | within each of those, against the ungrouped fit |
priors for the group-level SDs, and run_par_checks |
## Priors for the group-level standard deviations |
| the diagnostics and the failure modes | ## Failure modes, and being honest about them |
| the #33 route alongside | # Fitting each level separately, ## Crossed model weights |
| a dataset with genuine grouping structure | nassarius |
The dataset choice is better than it first looks. tank is within concentration — each tank has one dose — while contaminant is across concentration, spanning the full dose range. Those are the two structurally different cases, and having both in one dataset is what lets # Which kind of grouping is it? be a real decision rather than a definition. Restricting to survivors' growth to get a continuous response is a sensible simplification and is stated as one.
The JSS caution about hierarchical effects in a non-linear setting is carried into the closing section rather than asserted and dropped, as the queue asked.
Protocol compliance
vignettes/example7*untouched — Rewriteexample7as a growth-data case study using thealgadataset #193 is another session's.- Only
.Rmd.origis committed;precompile.Rwas not run and the rendered.Rmdis correctly absent. It renders under Re-run precompile.R before the next release: vignette output predates the current model code #190. - All nine chunks parse (
knitr::purl()→parse()). library(dplyr)andlibrary(ggplot2)are both already declared —dplyrinImports,ggplot2inDepends— andexample1/example6already usedplyrthe same way. I checked this specifically because a vignette dependency would not be caught by CI:R-CMD-check.yamlpasses--ignore-vignettes. No issue, recording the negative result.
⚠️ Filename collision — my error, and I am fixing it
This PR creates vignettes/example8.Rmd.orig. So does PR #238, the #219 complete-analysis-workflow draft I opened during phase 1, branched from issue-148-check-fit.
Since issue-6-33-grouping-vignette sits above issue-148-check-fit in the stack, both would land at the same path and conflict hard — two entirely unrelated vignettes.
This PR has the prior claim — it is older, it is part of the planned stack, and example8 follows naturally from the existing example1–example7. I picked the same name for the #219 draft without checking what the branches above had already taken, which was careless. I am renaming mine to example9 rather than asking this PR to move.
Nothing to do here; flagged so that whoever merges knows the conflict is being resolved on the other side and does not resolve it by dropping one of the two vignettes.
Smaller notes
# Choosing between the two routesis the section that justifies the queue's decision to cover Add the capacity to deal with random structure #6 and Add capacity for a factor covariate #33 together rather than separately, and it earns that decision. A reader who arrives wanting "a factor covariate" and finds two mechanisms needs exactly this.- The one thing I cannot check is whether the prose about what the terms do to the estimates matches what the fits actually produce — that is only verifiable once Re-run precompile.R before the next release: vignette output predates the current model code #190 renders it. Worth a read at that point rather than assuming the narrative survived.
Recommend merge, subject to the stack below it and to #238 renaming (which is on me).
Finding — the
|
| nested contrast | avoids conditioning | |
|---|---|---|
nassarius (current) |
yes | no |
herbicide |
no | yes |
Two ways to have both — RF's call
- Switch to
herbicide, keep the contrast as prose. Explain within- vs across-concentration grouping usingnassariusas a described example, one or two sentences and no fit, then demonstrate everything onherbicide. The teaching survives; the fitting is clean. - Keep
nassariusand model it honestly —bnec_hurdle()orfamily = "hurdle_gamma", so the deaths are in the model rather than filtered out. Keeps the nested structure and the real dataset, but makes a grouping vignette also a hurdle vignette.
My inclination is (1) — the vignette's job is grouping, and (2) asks the reader to learn two hard things at once. But there is a fair case that a grouping vignette which quietly sidesteps the hurdle question is doing the same thing as the filter it would be replacing.
Either way, note herbicide has 23 observations at or beyond the 0/1 boundary, so it needs Beta with the boundary handling check_data() applies — worth stating in the vignette rather than leaving to be discovered.
alga is not an option: #193 owns example7, which uses it.
Not changed pending RF's decision — this is a judgement about what the vignette should teach, not an implementation detail.
CI fixed — the failure was not this PR's subject matterEvery platform reported All three arguments were added by the phase 3 fixes themselves — decision (b) added
Also carried up from the base: No version bump — the monotone pinning is what keeps the merge path conflict-free. The restack merged with zero conflicts. |
Four findings from the review of this PR. group_estimate_table() read positions 1:3 of whatever the underlying method returned, while passing `...` straight through to methods that take `posterior` and `prob_vals`. nec(group_fit, posterior = TRUE) therefore reported draws 1, 2 and 3 as an estimate and its credible interval, and a longer prob_vals was silently truncated. Columns now come from the returned vector's own names, and posterior = TRUE is refused with a pointer at lapply(x$fits, nec, ...). This changes the default column names from Estimate/Q2.5/Q97.5 to Q50/Q2.5/Q97.5, which is what nec() itself returns per level. An NA in the grouping column passed every check -- factor() drops it from levels() and table() ignores it -- but data[grp == lev, ] is logical indexing with NA present, which puts an all-NA row into every level's subset. Refused. compare_posterior() moves down from #228, where it had been placed by mistake: it is package API for the class this PR introduces, so a reviewer of this PR should see it rather than finding it in a vignette branch. crossed_group_weights(pooled =) adds the queue's third reading -- does the factor matter at all. A pooled fit is scored on the same observations, so the grouped and pooled WAIC are directly comparable. The standard error needs the pointwise values, which do not survive every route a fit can take, so it is NA where they are absent rather than quietly omitted. Also corrects the rationale given last round: expand_manec() *does* record the weighting method, as attr(mod_stats$wi, "method"). It survives cbind and is dropped by row-subsetting, so capturing the request at fit time is still needed -- but the attribute is now read as a cross-check, since it reflects what the fit did rather than what was asked for, and levels weighted by different methods are refused. 02_deferred.md records the full seven-method toxval collision, verified against its NAMESPACE, and the methods this PR adds to that surface. test-bnec_group.R: 30 -> 55 assertions, all passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four findings from the review of this PR. group_estimate_table() read positions 1:3 of whatever the underlying method returned, while passing `...` straight through to methods that take `posterior` and `prob_vals`. nec(group_fit, posterior = TRUE) therefore reported draws 1, 2 and 3 as an estimate and its credible interval, and a longer prob_vals was silently truncated. Columns now come from the returned vector's own names, and posterior = TRUE is refused with a pointer at lapply(x$fits, nec, ...). This changes the default column names from Estimate/Q2.5/Q97.5 to Q50/Q2.5/Q97.5, which is what nec() itself returns per level. An NA in the grouping column passed every check -- factor() drops it from levels() and table() ignores it -- but data[grp == lev, ] is logical indexing with NA present, which puts an all-NA row into every level's subset. Refused. compare_posterior() moves down from #228, where it had been placed by mistake: it is package API for the class this PR introduces, so a reviewer of this PR should see it rather than finding it in a vignette branch. crossed_group_weights(pooled =) adds the queue's third reading -- does the factor matter at all. A pooled fit is scored on the same observations, so the grouped and pooled WAIC are directly comparable. The standard error needs the pointwise values, which do not survive every route a fit can take, so it is NA where they are absent rather than quietly omitted. Also corrects the rationale given last round: expand_manec() *does* record the weighting method, as attr(mod_stats$wi, "method"). It survives cbind and is dropped by row-subsetting, so capturing the request at fit time is still needed -- but the attribute is now read as a cross-check, since it reflects what the fit did rather than what was asked for, and levels weighted by different methods are refused. 02_deferred.md records the full seven-method toxval collision, verified against its NAMESPACE, and the methods this PR adds to that surface. test-bnec_group.R: 30 -> 55 assertions, all passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5e63942 to
232ed94
Compare
Handover note — state as of 2026-08-25, and the open questionPausing here. Everything below is on the branch ( The open question, which blocks the family choiceRF asked why the vignette fits What the paper did. Brinkman et al. (2023) analysed this dataset with Why But the switch does not work, for a reason nobody has explained yet.
Curve estimates stay stable and plausible throughout (NEC 900–955); the chains Three explanations were proposed and all three refuted by test: competing One observation that may or may not matter: the binomial families estimate What this means for the vignetteOn this dataset it appears to be either a working grouped demonstration It is also material to #250, which claims group-level terms work for bounded Branch state
A rendered review copy of the pre-switch (Beta) vignette, with all eight Two side findings worth their own issues
Still open from earlier, unchanged
|
#257 was the untested explanation, and it accounts for most of the divergences — but not all of themThe handover note records three explanations proposed and all three refuted by Run on this vignette's own call and its own data, so the comparison is
What this settlesThe mechanism is confirmed. Divergences fall 63-fold for What it does not settle, and this is the part that matters for the vignetteNo arm converges. The package screens on The two families disagree about the NEC. 959 against 1537, on the same The design is thin for what it is being asked to do. 150 rows across 30 Suggested readingThe family question is no longer the blocker; the design is. Two options that do
Both are RF's call. Recorded here rather than acted on. Measured on |
Rebuilt from current dev. Only vignettes/example8.Rmd.orig is taken from f7f3d75; every R file on that branch is superseded by dev, and the coral dataset is held back pending the redistribution decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JnAiNVJnKrdpa1PwQP2kCm
Shows the brms formula ogl(), (par | group) and pgl() each produce from nec4param, generated by make_brmsformula() rather than transcribed, under Beta(link = "identity") and gaussian(). Adds the design mapping for each term and the behaviour of a term naming a parameter an equation lacks. Data supplied by one swappable chunk on herbicide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JnAiNVJnKrdpa1PwQP2kCm
f7f3d75 to
815b80c
Compare
|
Branch rebuilt and body updated. A review posted against the earlier text was The branch was cut fresh from The diff is now Two changes of substance since the last review:
Every stated dependency in the old body is now on 🤖 Generated with Claude Code |
Ritz on normalising to a control, and Luter as the source of the dataset the grouping vignette is being rebuilt on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JnAiNVJnKrdpa1PwQP2kCm
|
Dataset decision, 2026-09-09. The vignette is rebuilt on the acute copper and zinc data from Luter et al. It is the only dataset screened that supplies all three structures the vignette Why this dataset rather than a structurally adequate oneIts published analysis divides each plate by the median of four control wells and Under Other case studies retained
Dropped
Provenance and redistribution status for every dataset screened is recorded in Two questions are outstanding with Heidi Luter and block nothing: whether the 🤖 Generated with Claude Code |
Acute copper and zinc tests from the Lum-31 bioluminescent bacterial assay of Luter et al. (2025), for the group-level term vignette (#6, #33). The one blank-corrected negative the source workbook did not floor is kept as recorded, so censoring is flagged on rlu <= 0 rather than rlu == 0. The censoring bound is the smallest positive reading per plate, pooled over the two exposure times because the two reads of a plate share a gain setting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VaApqBU7nmeG2pnu1wEsVA
The four wells at one concentration on one plate, which is the within-concentration grouping factor for the example8 section 4 fits. A group-level term on well is an observation-level term within one arm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VaApqBU7nmeG2pnu1wEsVA
…IMS/bayesnec into issue-6-33-grouping-vignette-v2
They produced the measurements the vignette sections are written against, and re-running a comparison after the prior, dispersion and backend changes means editing a formula rather than reconstructing the script. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VaApqBU7nmeG2pnu1wEsVA
Status, 2026-09-11The branch now has the dataset, a drafted vignette and the exploratory fits that What is on the branch
What the fits establishedThree results the vignette is written around, all measured on the zinc Normalisation is unavailable here, not merely inadvisable. Dividing by the A low ECx tracks model-set composition; EC50 does not. Among the equations The constant-dispersion Gamma overstates the variance at the control about What it is blocked byFour decisions, none of which I can take.
And three changes that make the current numbers provisional. #304 changed the What a fresh session needsRead in this order. Fits are at Settings that are not obvious. Two things measured here that belong to other issues, already commented Tasks 6, 7 and 8 are untouched: the style pass over the older sections, the |
Removes an 'it is worth asking' construction. Headings in the retained sections are all noun phrases; the five violations the plan recorded were in the coral sections, which the rewrite replaced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VaApqBU7nmeG2pnu1wEsVA
Flores et al. (2021), the diuron by climate scenario experiment the JSS article cites for compare_posterior(). Five concentrations in the colour endpoint is the conventional minimum, which is what section 6 needs; the yield endpoint gives the well-conditioned contrast at 54 chambers. Bibliography fields confirmed against Crossref rather than inferred: Marine Pollution Bulletin 169, 112582, doi 10.1016/j.marpolbul.2021.112582. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VaApqBU7nmeG2pnu1wEsVA
The lum31 fits gain disp("power"): measured, a constant-CV Gamma puts about
eight times the observed variation at the control. The coral and herbicide fits
keep a constant dispersion, nothing having been measured about theirs.
Section 7 gains the lum31-by-toxicant arm, fitted with the structure sections 3
to 5 arrive at rather than on pooled plates.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VaApqBU7nmeG2pnu1wEsVA
crf() resolves its model argument in the global environment only, so a variable holding the set fails under knitr and inside any function. The precompile of example8 errored at the first fit with 'object eqs not found'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VaApqBU7nmeG2pnu1wEsVA
chains = 4, iter = 5000, warmup = 4000: half the bnec() default iterations and half the retained draws, keeping the 80 per cent warmup ratio. At 16000 draws the lowest effective sample size measured on any contributing equation was 2856, so 4000 draws has measured headroom over the conventional floor of 400. The vignette now fits 189 models, which scales to 10 to 20 hours against the 24 hour walltime, so that is raised to three days. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VaApqBU7nmeG2pnu1wEsVA
|
Body updated to describe the current change rather than the state at branch rebuild. What moved: the diff is no longer two files --- it now adds three datasets ( Precompile job 896331 is running on the HPC with the full retained set at |
…gnette-v2 # Conflicts: # DESCRIPTION
bnec() reports an equation it could not fit and returns the fit for the rest, and try() prints that as '#> Error : Failed to fit model <name>.'. Matching on '^#> Error' refused to ship example8 after 23 h of fitting, when every chunk had succeeded and only ecxhormebc5 had failed to initialise. The exemption is written to that exact text: 'None of the models fit successfully' and '#> Error in ...' still fail, and reported failures are announced rather than passed over. Also plot() rather than autoplot() for a bayesnecgroupfit, which has no autoplot method. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015qr9z6XEX1vxU6v18KCXmF
Since #184 a set is fitted under a future plan. precompile.R sets one where BAYESNEC_VIGNETTE_WORKERS is given, so every other vignette is rendered as before and #190 is not silently a different run; precompile-hpc.sh gains --workers to set it. The allocation rises to 16 cores and 128GB. Sixteen because the parallelism is inside bnec() and bnec_group() fits its levels in sequence, so the useful width is the equations in one call: example8's sets are 15 to 18. The 23 h run of 2026-09-12 should fall to about 6 h. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015qr9z6XEX1vxU6v18KCXmF
A future plan is off by default, so the directives stay at 4 cores and 32GB and precompile-hpc.sh overrides both on the sbatch line when --workers is given. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015qr9z6XEX1vxU6v18KCXmF
The herbicide arm used decline to reproduce the JSS article's final figure and the toxicant arm used all, which varied the set between two calls being compared in one section. Also removes a duplicate ritz2026 bib entry and adds fisher2024, which the bibliography did not have. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Zinc rises above its control at low concentrations, so decline would exclude equations the data support. The evidence is computed in the vignette rather than asserted, and the two bnec_group calls now state why their sets differ. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
refresh = 0 on every fit: cmdstanr writes progress to stdout where rstan wrote it to stderr, and the render reached 7.2 MB with 26 per cent of its lines sampler progress. Section 4 reads the group-level standard deviation it claims, which summary.bayesmanecfit does not report, and states that it depends on the equation. The N(S)EC is on the log(conc) scale where ecx() is back-transformed; the section now says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
compare_posterior() returns the draws it compared, in $posterior_list and $diff_list, so printing the object dumped 4000 draws for each level: 112122 of the rendered vignette's 115806 lines, and 5.5 MB against 14 to 72 KB for every other vignette. $prob_diff is the table the JSS article shows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011RsKEWCNtNVoHEpFzeFLkB
…gnette-v2 # Conflicts: # .gitignore # DESCRIPTION # vignettes/precompile.R
rhat() returns one value per parameter, so printing it listed all 45 chamber effects for each of four equations -- hundreds of lines that do not answer the section's claim about three against four curve parameters. The chunk now reports divergent transitions and maximum R-hat per equation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011RsKEWCNtNVoHEpFzeFLkB
Numbers are from job 902840, a production run at the same seed and settings, and are provisional: that render predates the 221-commit merge of dev, so every value is verified against the render now running (910760) before this is called done. Sections 4 and 6 wait on that render for the group-level standard deviation and the divergence counts respectively. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011RsKEWCNtNVoHEpFzeFLkB
Two of the nine fitted objects were plotted. The rest now are: both section 4 fits, the pooled section 5 fit beside the plate one, the coral yield fit, the three- against four-parameter pair, and the toxicant group. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011RsKEWCNtNVoHEpFzeFLkB
Closes #6. Documents #33's
bnec_group().What
Adds
vignette("example8")on group-level terms and factor covariates, and thethree datasets it is built on:
lum31,coral_colourandcoral_pam.The vignette works through what
ogl(),pgl()and(par | group)each do tothe equation, when a grouping admits only a displacement and when it admits a
difference in shape, and when levels are fixed and of interest in themselves and
so belong to
bnec_group()instead.Why it matters
No vignette or manual page states what
brmsformula each of the threegroup-level syntaxes produces, so a user choosing between them has to read the
source. Nothing states which one a given design admits, and the commonest error
--- placing a random structure across levels that are fixed and of interest ---
is not addressed anywhere.
The case study gives a reason to use a group-level term that does not depend on
the reader already wanting one. Dividing a response by its control mean is
standard practice in ecotoxicology, and Ritz et al. (2026) show it biases effect
concentrations downwards and gives intervals that are too narrow. A group-level
term on the control parameter targets the same quantity with the uncertainty
propagated, so the grouping machinery is the answer to a problem users already
have.
Evidence
Three findings from the exploratory fits decide what the sections claim. All are
recorded with their settings in
prompts/grouping-vignette-dataset.md, and thescripts that produced them are in
notes/scripts/example8/.Normalisation is unavailable on this assay rather than merely inadvisable.
The plate reader sets its gain per read, so dividing by the largest reading in
the dataset leaves plate controls between 0.171 and 0.898 of scale. What makes a
normalised analysis work is every curve's control landing near the top of a
bounded scale, and that needs a per-curve divisor --- which is what Ritz et al.
object to.
A low ECx tracks model-set composition; EC50 does not. Among the equations
holding weight, EC50 spans 17 per cent and EC10 spans a factor of four. Each
equation's own EC10 is stable across plate structures to within 10--30 per cent,
while the model average moves from 0.662 to 0.287 mg/L.
A constant-dispersion Gamma overstates the variance at the control about
eight-fold. Observed within-cell CV runs 0.051 at the control to 0.362 at the
bottom; the fitted constant is 0.43.
disp("power")brings the implied controlCV to 0.079, and both it and an unconstrained smooth agree there.
Implementation detail
The datasets
lum31--- acute copper and zinc tests from the Lum-31 bioluminescent bacterialassay of Luter et al. (2025). 2904 rows, 33 plates, five batches. Replication at
three scales:
conc_group(four wells at one concentration on one plate),plate(a complete series), and toxicant by exposure time.rluis reported asrecorded, including one blank-corrected negative the source did not floor, which
is the evidence that the zeros are floored negatives rather than absences of
light;
censoringandrlu_censgive the left-censored form.coral_colourandcoral_pam--- two endpoints of one diuron by climatescenario exposure, Flores et al. (2021), the study the JSS article cites for
compare_posterior(). Five concentrations in the colour endpoint is theconventional minimum, which is what section 6 needs; the yield endpoint gives the
well-conditioned contrast at 54 chambers.
Source workbooks and CSVs for all three are git-ignored under
data-raw/.Redistribution is recorded in
notes/dataset_provenance.md.Settings
The vignette fits 189 models:
model = "all"for thelum31andcoral_pamarms,
"decline"for the herbicide arm so it reproduces the JSS article's finalfigure, and deliberate two-equation subsets for section 6's parameter-count
contrast. Chains are 4 at
iter = 5000, warmup = 4000--- halfbnec()'sdefault iterations and half its retained draws, keeping the 80 per cent warmup
ratio. At 16000 draws the lowest effective sample size on any contributing
equation was 2856, so 4000 draws has measured headroom over the conventional
floor of 400.
adapt_deltais 0.99 throughout. At the default,ecxll5gave 880 divergenttransitions of 16000 while holding a fifth of the weight; fitted alone it gives
0 at 0.99, so the cause is the step size rather than the design.
Precompiled on the HPC under #308.
hpc/run.precompilehas its walltime raisedfrom 24 hours to three days, because 189 fits scale to 10 to 20 hours against a
24-hour limit. Every vignette inherits that; say so if it should be per-run.
Known limitations
The model set is written out as
"all"rather than held in a variable because of#319 ---
crf()resolves its model argument in the global environment only, so avariable set fails under
knitr.