Open IndrajeetPatil opened 3 years ago
Though I think the closer equivalent of broom::augment would be directly get_predicted()
, modelbased is really a high level package that closely integrates plotting etc.
broom::augment()
is somewhat confusing in that it does three things in one function:
The applications of all of these are mostly for various types of plotting (e.g., for diagnostics, evaluating model fit, or illustrating predictions). Indeed, I think the main reason that augment()
exists the way it does is because of its historical connection with ggplot2::fortify()
.
It makes some sense to me that (1) and (2) would use the same function (as modelbased
does), but I don't really like including the diagnostic statistics in the same function. This is especially the case because the types of residuals that are useful for things like evaluating model performance [(1) and (2)] are not usually the residuals that should be used for model diagnostics (e.g., weighted/Pearson residuals, not simple observed − predicted residuals).
Based on my reading of various discussions, the current easystats thinking is that modelbased
should be the home for (1) and (2) but performance should be the home for (3).
totally agree
@vincentarelbundock Given that you are the team member with the most experience of contributing both to the broom and easystats ecosystems, I am wondering if you would be interested in jump-starting this article?
No worries if you are already spread too thin to take an additional thing on your plate. Just wanted to check ☺️
Thanks for the ping @IndrajeetPatil!
Honestly, I don't feel I have the energy to write a polished vignette on this, but here are a few points I think are noteworthy.
broom
and easystats
The broom
package has three main functions. The easystats
analogues are spread over several packages.
tidy()
extracts the model's parameters: coefficients, standard errors, confidence intervals, etc.
easystats
analogue: parameters::parameters
glance()
computes goodness-of-fit statistics and performance metrics
easystats
analogue: performance::performance
augment()
adds residuals and fitted values to the data frame used to fit the model.
augment()
, but the insight::get_predicted()
function is a more powerful way to get fitted values.broom
returns standard data frames with a consistent naming convention. easystats
returns special classes of objects which can be pretty-printed to the console, or converted to data frames using as.data.frame()
.
The column names used by broom
and easystats
functions are not identical, and by default easystats
pretty-prints the results. However, the output can be normalized as follows:
library(broom)
library(parameters)
library(performance)
mod <- lm(mpg ~ hp + factor(cyl), data = mtcars)
# default output
parameters(mod)
performance(mod)
# broom-style output
mod |>
parameters() |>
standardize_names(style = "broom")
mod |>
performance() |>
standardize_names(style = "broom")
easystats
over broom
easystats
supports a lot more models than broom
out of the box.broom
is now in maintenance mode and will no longer be actively developed.easystats
relies on fewer dependencies.easystats
does more code re-use. Default methods often work automatically.easystats
has way more features than broom
. Too many to list here. See: https://easystats.github.io/easystats/broom
over easystats
broom
because each model is associated with 3 very simple and transparent S3 methods. In contrast, the output of an easystats
call is often generated by a combination of many function calls.broom
is often much faster than easystats
broom
vs. easystats
These (controversial?) takes are 100% based on my current feels. I have no actual checklist of stuff to work on, but am just listing a few areas of concern, where easystats
may be a bit more vulnerable than broom
.
broom
focuses almost exclusively on accessing information from models, or on reporting the output of functions supplied by the modeling package. In contrast, easystats
offers a lot more features, and a lot of those features require easystats
to make "original" calculations. That's fine, and so far whenever I dove deep, things looked good. That said, I do feel that some code is "risky", and that the test suite is too focused on "internal" consistency, and not focused enough on finding "external" benchmarks to compare our results to. It's important to make sure our results don't change from commit-to-commit, but a lot of the numerical tests are hard-coded instead of "comparative" to some other package.
As mentioned above, my experience is that broom
is often much faster than easystats
. That is to be expected, because there is a lot more processing of all the objects. But I do think we should make a concerted effort to benchmark and grab all the low hanging fruit we can in terms of performance.
Feature creep scares me a lot, and I worry that easystats
may become unmaintainable because of life changes for a few of the core contributors. broom
is a super widely used package, and it's supported by RStudio, and yet it has decided to go into maintenance mode because the scope became unmanageable. The scope of easystats
is much bigger than broom
...
This is incredible, @vincentarelbundock! Thank you so much for your valuable (and incredibly insightful) remarks!
Your comment actually provides the perfect skeleton, wherein I can now introduce some flesh. ✍️
@easystats/maintainers Please have a look at Vincent's comment. I think these are valid concerns, and we should pay heed to them and make the necessary changes.
Thanks a lot for your input, Vincent. I share your concerns you mentioned under "Further thoughts about broom vs. easystats".
Regarding 2), I think we managed to address many / most important points where performance was really poor and could improve the speed a lot. Still, we have to keep this in mind.
Regarding 1), that's why I don't like snapshot tests. I'd say there are quite a lot of places in our tests where the hard coded values are indeed from external sources, so a kind of "external validation". And in some places, in particular get_predicted()
, we test a lot against external methods. But yes, tests can still be improved across all packages.
And regarding 3), I think we have made some improvements on the one hand; on the other hand, since we're adding new features regularly, we always fall behind our achievements again ;-) As an example for parameters, we could "templatize" the code files a bit more: e.g., if the methods_AER.r file has no (needs no) model_parameters()
method, we could still add a comment as placeholder, so each file follows the same pattern. Furthermore, we should add more comments and make parts of the code clearer for people who are not familiar with the code base. Vincent has done this a lot, also because he needed to do so in order to contribute ;-) Same for @etiennebacher, who has done a lot of great contributions in the past (and a lot of others who contributed to the easystats eco-system). So maybe from your perspective, you could make some suggestions how we can make the code files clearer and easier to understand (I don't mean refactoring now, that could be done in a later step, but simpler things like adding comments, adding "comment-sections" to structure the code etc.).
Regarding all the previous part of Vincent's post, that's a great draft we can build on.
Regarding 3) I agree feature creep is scary, but my hope is that we will (soon) reach a point of stability, especially for lower-level packages like insight/datawizard/parameters, where the methods will work and be feature-complete for 98% of the most used models. Since we don't rely much on other dependencies, we don't have to adjust to upstream changes, so we will enter a maintenance phase where most of the additions will be related to edgecase bugs, and the supporting of new models as they are made. And less breaking changes :) But it's true that, despite having this as a general direction and end-goal, it's hard to tell through which port the easystats ship will sail, given that we must also stay aware of the tides and the winds that blow in the R-verse, to keep our momentum and stay relevant
Regarding the code quality yes, there's a lot improvement to be made in terms of efficiency or simply structure (RIP bayestestR ). But for that, the best would be to obtain funding and recruit a dev that could help with that as a full-time job. In general, we should really start seriously looking for funding, so many smaller projects are funded I don't see why not easystats
Thanks all for engaging! All your points are well-taken and reassuring. To be clear, none of the points I raised seem like big deals to me. The code base is nice and relatively easy to read, and there are many tests. I just wanted to flag these as things to keep in mind for the future, but IMHO the current state of affairs is healthy and exciting!
The idea of finding grant support is very intriguing. I don't know of any programs like this in Canada, but would be very curious to learn about them (here or elsewhere).
More thoughts and questions (as the maintainer of broom.mixed
).
I would be interested in bringing broom.mixed
and parameters
closer together/not unnecessarily duplicating efforts, and in taking a closer look/comparison of the mixed-model methods to see how the feature sets compare specifically for this domain.
(Based on GH activity it seems as though broom
is back in development mode, supported by the tidymodels group ... ??)
There might be philosophical differences in some places as well (e.g. I'm really not sure about providing Wald CIs, even on the log scale, for RE SDs by default, although I agree that it's a useful option to have ...)
Another question/thought is about how easy it is to facilitate contributions of additional methods ... for better or worse, tidy
output is very lightweight. (I might personally use a helper function that did
model_parameters(model) |> c() |> tibble::as_tibble()
possibly optionally renaming the columns for broom
-compatibility ... it feels like broom
is slightly more workflow-oriented while parameters
is slightly more display-oriented?
library(broom.mixed); library(parameters)
m1 <- methods("model_parameters")
m2 <- methods("tidy") ## 157 but includes broom stuff as well
m3 <- broom.mixed::get_methods() ## 24
p_methods <- m1 |> unclass() |> c() |> gsub(pattern = "^[^.]*\\.", replacement = "")
b_methods <- m3$class
intersect(p_methods, b_methods)
## [1] "brmsfit" "gamlss" "glmmTMB" "lme" "lqmm" "mcmc"
## [7] "mcmc.list" "MCMCglmm" "merMod" "MixMod" "rlmerMod" "stanfit"
## [13] "stanreg"
setdiff(b_methods, p_methods)
## [1] "allFit" "gamm4" "glmmadmb" "gls" "lmList4"
## [6] "mediate.mer" "ranef.mer" "rjags" "TMB" "varComb"
## [11] "varFunc"
I don't know exactly how to isolate "mixed model-like classes" from the list of model_parameters
methods ...
This is the list of model_parameters
methods for classes that are the same as the name of a package listed in the MixedModels
task view ...
mm_pkgs <- ctv:::.get_pkgs_from_ctv_or_repos("MixedModels")
intersect(p_methods, mm_pkgs[[1]])
[1] "bamlss" "blavaan" "gamlss" "ggeffects"
[5] "glmm" "glmmTMB" "hglm" "lavaan"
[9] "lqmm" "marginaleffects" "MCMCglmm" "mmrm"
[13] "sem"
I would be interested in bringing broom.mixed and parameters closer together/not unnecessarily duplicating efforts
I can’t speak for the core devs, but from the perspective of a user (who sometimes makes minor contributions), this would be a welcome development. A couple comments.
First, from a design perspective, I feel that the main differences
between parameters
and broom
are:
parameters
allows more code re-use by splitting functions into p_value()
, se()
, etc. In contrast, broom
tends to repeat everything in every method. This is a trade-off between code re-use and the simplicitly of supporting new models.parameters
does not supply an equivalent to glance()
. Those extractors are held in a separate package: performance
. https://easystats.github.io/performance/parameters
does not depend on tidyverse
.FYI, you can rename columns to broom
styles as follows:
library(parameters)
mod <- lm(mpg ~ hp * am, data = mtcars)
parameters(mod) |> standardize_names(style = "broom")
term estimate std.error conf.level conf.low conf.high
1 (Intercept) 26.6248478696 2.18294320 0.95 22.15329144 31.09640430
2 hp -0.0591369818 0.01294486 0.95 -0.08565332 -0.03262064
3 am 5.2176533777 2.66509311 0.95 -0.24154237 10.67684913
4 hp:am 0.0004028907 0.01646022 0.95 -0.03331435 0.03412013
statistic df.error p.value
1 12.19676624 28 1.014017e-12
2 -4.56837583 28 9.018508e-05
3 1.95777527 28 6.028998e-02
4 0.02447662 28 9.806460e-01
model_parameters(model) |> c() |> tibble::as_tibble()
An alternative syntax and strategy would be to simply define a tidy.parameters_model()
method to get:
model_parameters(model) |> tidy()
I will take a first crack at this once the new
API
formodelbased
is stable (https://github.com/easystats/insight/issues/315).Should discuss:
easystats
overbroom
broom
code toeasystats
codeFeel free to add comments listing other things to discuss.