Repository navigation
Forward merge develop into dev-1.5.0 - #521
Merged
Merged
Conversation
A softmax fit of data generated by Gaussian noise reports set-size effects on c and a that do not exist and changes c/a, so users need the Gaussian rule to check whether a conclusion depends on the noise distribution. The rule keeps the m3() API and the native multinomial family: each category's non-linear formula calls a generated Stan function m3_gauss_logp() that returns log P(k) by 40-node Gauss-Hermite quadrature, and softmax of log-probabilities that sum to one returns them unchanged. log_lik(), posterior_predict(), posterior_epred() and pp_check() go through brms as for softmax; brms evaluates the formula in R through the exported companion m3_gauss_logp() (keywords internal, the sdt_rating_logmu() pattern). dm3()/rm3() use the same R kernel. 40 nodes instead of 72: maximum-likelihood estimates on pooled oberauer_lewandowsky_2019_e1 move by 2e-4 standard errors, at 40/72 of the cost per gradient. Default priors for c and a are normal(2, 1) (equal means as for softmax, centred on the Gaussian scale); by per-subject posterior mode on O&L e1 they pull estimates less than softmax's normal(3, 1) does under softmax (median |MAP - MLE| 0.44 vs 0.57 SE for intercepts). b is fixed at 0. choice_rule is now lower-cased before the version tables are read.
Adds the rule's probability integral, cites Yellott (1977) for why softmax and Gaussian are different models, and says that c-vs-a comparisons and effects of the number of candidates can differ between the two rules.
…cdf fallback engages
… default priors per rule
… on the multinomial route conditional_effects() without `effects` warned "NaNs produced" for parameters of some multinomial models (m3 with dist as the first category, two-tree MPT models), also for parameters with no predictors. The route asked brms for the grid of the first category dpar over every effect of the model and kept only the parameter's own afterwards. For an interaction of two numeric columns brms holds the second at mean - sd, which is below 0 for a 0/1 indicator or a count like n_dist, and the dpar takes its log; the element was discarded, but the warning reached the user. The grid call now gets the parameter's own effects as `effects`, taken from brms's default list in brms's order and with the numeric variable of a pair first, as brms orients it only for its default list. A parameter without own effects calls brms no more and still resolves its draws, so the result and the draws of the next parameter are unchanged. A user's `effects` reaches brms as before. Tests: the grid call receives only the own effects and none for a parameter without effects; the default list and its orientation agree with brms's own conditional_effects() run with the evaluation stubbed; the draw ids of the next parameter are unchanged; a real m3 fit with dist first gives no warning and the same result as effects = "cond". The selection tests now call .ce_select_own_effects() on a list of effects, with their expectations unchanged.
feat(m3): add choice_rule = "gaussian", a Thurstonian choice rule
…rticle-fits release (#438)
fix(conditional_effects): ask brms only for a parameter's own effects on the multinomial route
The check diffed BASE...HEAD, so a fit added in one commit and deleted in a later one passed, yet a merge commit carries the blob into develop for good. The step now lists the objects reachable from the PR head and from neither the base nor origin/develop, and fails on every blob above the limit, naming its path and the commit that added it. Blobs already in develop are exempt, so a dev-* PR whose head merged develop does not fail on grandfathered fits. The blobs the blobless clone lacks come in one fetch. Looked up one by one, each blob arrives in its own pack, about 0.4 s each, and the lookup aborts after about 50 packs.
… release (#438) When the article-fits release does not exist, the download step continues with no fits, and a fit that an article loads from fits/ but the release lacks is fitted during the site build. A new step lists every file = "fits/<name>" in vignettes/articles/*.Rmd after the download and fails, naming the article and the fit, when a listed fit is absent or readRDS() cannot read it. A fits/ path that is not a string literal passed to file fails too, because it cannot be listed.
…ase is created (#438) The workflow runs on release: published, which also fires for a pre-release, and under that event the job builds the commit of the tag and deploys it to gh-pages. The article-fits release only holds fits, so the job is skipped for that tag. Versioned releases deploy as before.
The section now says that the file-size check reads every commit of the PR, not only the final tree; that files already in develop do not count; and that the site build stops, naming the article and the fit, when a fit loaded from fits/ is missing from the release. It also asks for the path as a string literal, which the build needs to list the fits.
brms stores the init function in the fit, and saveRDS() writes its environment. Built inside create_initfun.default(), the closure kept the method's frame, whose generic call environment leads back to the frames of bmm() or update() with the data and the fitted model, so every saved fit carried extra copies of both. The closure is now made in a helper whose frame holds only the Stan parameter list, the Stan data and the model, forced before the closure is returned.
…ory; run on retarget (#438) An independent review found three gaps in the scan. - Excluding the base and develop with ^rev drops only the trees of their tips, so a PR that restores a fit that develop had deleted earlier failed although the blob is in the protected history. The step now removes every object reachable from the base or from develop. - A stacked PR is retargeted to develop when its base merges, which is an edited event, so the check never ran for it. edited is now a trigger type. - Paths with a comma, colon or percent sign broke the error annotation.
#438) An independent review found fits the step could not see and references it rejected for no reason. - Read every vignettes/**/*.[Rrq]md, as pkgdown does, and look for the fit next to the article. - Follow knitr's fences: chunks in blockquotes count, chunk syntax shown in a longer fence does not, chunks with eval = FALSE are skipped. - Fail on inline code that names a fits/ path. - A bare "fits" string counts only inside a file argument, so .key = "fits" no longer fails; `file` in backticks is recognised. - Invalid UTF-8 in an article no longer makes the step see nothing.
A merge commit keeps every commit of a PR, a squash merge does not; files held anywhere in the history of develop or of the base do not count; the site build reads file = "fits/<name>" in R chunks only.
cat-file prints '<oid> missing' for an object the batched fetch did not deliver, and the size loop skipped such a line, so an unsized candidate passed unchecked. The step now reports the count and fails. The annotation names the commit by its first ten hex digits (--no-abbrev also disabled %h), and the comment on the edited trigger says it fires for title and body edits.
The R code of the "Check article fits" step moves unchanged to .github/scripts/check-article-fits.R, so that it can be parsed, linted and run on fixtures with Rscript.
…ath (#438) The fit check's chunk scanner now applies knitr 1.52's rule: plain fences are text, a chunk header opens a chunk wherever it stands, only a fence with the header's own prefix and backtick count closes it, an unclosed chunk runs to the end of the file, and #| options are read from the leading lines only. A chunk shown inside a four-backtick block, a chunk after an unclosed plain fence, an unclosed chunk and #| eval: false after code were evaluated by knitr but passed the check. The check skips the files pkgdown does not build (names starting with _, vignettes/tutorials/), checks every literal relative file = path, so a fit in assets/ that is not in the tree fails too, ignores saveRDS(), save() and write_rds() calls, and escapes its annotations.
A mutant that also keeps the checked data in the closure passed both earlier tests, while it put every unused column back into saved fits. The update test's comment now says that brms stores update()'s init function only on a recompile, the one route the mock backend runs.
update() without recompiling keeps the old fit's init function, so an old fit keeps its extra copies through it (#518).
# Conflicts: # .github/CONTRIBUTING.md
…es/ is absent (#438) The fit check treated every literal file = as a fit and appended .rds, so read.csv(), write.csv(), png(), cat() and readRDS(file = "x.RDS") failed the build. It now checks only bmm(), brm(), update(), readRDS() and read_rds(), appends .rds only where bmm() and brm() do, ignores writers, and still fails on any fits/ path it cannot check. Without a vignettes/ folder it stops instead of reporting 0 problems.
Saved fits no longer carry extra copies of the data and fit
CI: size check for added files and release-hosted fits (#438)
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.
Forward merge (ADR 0002). Brings file-size.yaml and the article fit check (#517) to dev-1.5.0.