Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new vignette to catalog comparisons between broom and easystats ecosystems #104

Open
4 tasks
Tracked by #113
IndrajeetPatil opened this issue Mar 8, 2021 · 12 comments
Open
4 tasks
Tracked by #113
Assignees

Comments

@IndrajeetPatil
Copy link
Member

I will take a first crack at this once the new API for modelbased is stable (easystats/insight#315).

Should discuss:

  • Differences in APIs and philosophies
  • Advantages and disadvantages (?) of easystats over broom
  • How can you convert existing broom code to easystats code
  • How can they co-exist peacefully in a single script

Feel free to add comments listing other things to discuss.

@IndrajeetPatil IndrajeetPatil self-assigned this Mar 8, 2021
@DominiqueMakowski
Copy link
Member

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.

@bwiernik
Copy link
Contributor

broom::augment() is somewhat confusing in that it does three things in one function:

  1. Extract fitted model data and predictions.
  2. Make predictions for new data.
  3. Extract observation-level diagnostic statistics.

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).

@DominiqueMakowski
Copy link
Member

totally agree

@IndrajeetPatil
Copy link
Member Author

@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 ☺️

@vincentarelbundock
Copy link

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.

Equivalence between broom and easystats

The broom package has three main functions. The easystats analogues are spread over several packages.

  1. tidy() extracts the model's parameters: coefficients, standard errors, confidence intervals, etc.
    • easystats analogue: parameters::parameters
  2. glance() computes goodness-of-fit statistics and performance metrics
    • easystats analogue: performance::performance
  3. augment() adds residuals and fitted values to the data frame used to fit the model.
    • AFAIK, there is no direct analogue to 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")

Advantages of easystats over broom

Advantages of broom over easystats

  • It is arguably easier to add support for new models in 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

Further thoughts about 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.

  1. 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.

  2. 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.

  3. 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...

@IndrajeetPatil
Copy link
Member Author

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.

@strengejacke
Copy link
Member

strengejacke commented Jul 31, 2022

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.

@DominiqueMakowski
Copy link
Member

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

@vincentarelbundock
Copy link

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).

@bbolker
Copy link

bbolker commented May 8, 2024

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"     

@vincentarelbundock
Copy link

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:

  1. 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.
  2. parameters does not supply an equivalent to glance(). Those extractors are held in a separate package: performance. https://easystats.github.io/performance/
  3. 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

@vincentarelbundock
Copy link

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()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants