Repository navigation
[Bug]: brmsfit .parameters_starting_text() tests attribute values with %in% which is wrong and very slow #627
Description
Describe the bug
.parameters_starting_text() (report 0.6.4) starts with:
if ("pretty_names" %in% attributes(params)) {attributes() returns attribute values, so this has two problems:
- Always FALSE. A
pretty_namesattribute is never detected. - Slow.
match()coerces every attribute value to character. For Bayesian models,
bayestestR::sexit()stores the whole fitted model inattr(params, "data"), so the model is
coerced on every call, and the cost grows superlinearly with model size.
I discovered this on a brms model, with 814 phylogenetic random effects and 12,000 draws (data attribute 176 MB), had not finished after 100 minutes.
Fix:
if ("pretty_names" %in% names(attributes(params))) {Reprex:
Base R code shows the issue:
- 0.01 s for 1e4 calls of
attr(, "data") - 0.25 s for 1e5 calls of
attr(, "data") - 16.8 s for 1e6 values in
attr(, "data")
And applied to a brms model: y ~ x + (1 | g), 400 groups, 2 chains × 3000 iterations: report() takes 133 s in report 0.6.4 but only 37s with the fix.
Versions: report 0.6.4, bayestestR 0.19.0, brms 2.23.0, R 4.6.2, macOS 15.8 (aarch64).
(Claude was used to help me debug this and write a reprex)
Reproducible example
# report 0.6.4: report() on brmsfit slows superlinearly with model size.
# Cause: report:::.parameters_starting_text() tests
# "pretty_names" %in% attributes(params)
# which match()es against every attribute *value*. bayestestR::sexit() stores
# the whole model in attr(, "data"), so the model is coerced to character.
# The test is also always FALSE; intended: names(attributes(params)).
library(brms)
# 1. base R: wrong result
params <- data.frame(Parameter = "b_x")
attr(params, "pretty_names") <- c(b_x = "x")
"pretty_names" %in% attributes(params) # FALSE
"pretty_names" %in% names(attributes(params)) # TRUE
# 2. base R: cost of the test grows with the size of attr(, "data")
for (n in c(1e4, 1e5, 1e6)) {
attr(params, "data") <- list(draws = matrix(rnorm(n), ncol = 10))
print(system.time("pretty_names" %in% attributes(params))[["elapsed"]])
}
# 3. end to end: a modest brms model
set.seed(1)
d <- data.frame(g = factor(rep(1:400, each = 3)), x = rnorm(1200))
d$y <- d$x + rnorm(400)[d$g] + rnorm(1200)
m <- brm(y ~ x + (1 | g), data = d, chains = 2, iter = 3000, seed = 1, refresh = 0)
t_bug <- system.time(report::report(m))[["elapsed"]]
# patch: the one-token fix
f <- report:::.parameters_starting_text
src <- paste(deparse(body(f)), collapse = "\n")
stopifnot(grepl('"pretty_names" %in% attributes(params)', src, fixed = TRUE))
body(f) <- str2lang(sub('"pretty_names" %in% attributes(params)',
'"pretty_names" %in% names(attributes(params))', src, fixed = TRUE))
assignInNamespace(".parameters_starting_text", f, "report")
t_fix <- system.time(report::report(m))[["elapsed"]]
cat(sprintf("report(): %.1fs as shipped, %.1fs with names(attributes(params))\n", t_bug, t_fix))
Activity
mattansb commentedon Nov 3, 2026
Hey @SimonGreenhill wanna make a PR of this?