-
-
Notifications
You must be signed in to change notification settings - Fork 23
estimate_grouplevel() gains a new "marginal" method for estimating group-level parameters #574
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
Conversation
Summary of ChangesHello @DominiqueMakowski, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new "marginal" method to estimate_grouplevel() for estimating group-level parameters using marginal means and effects rather than raw random effects. The feature provides improved interpretability by computing random intercepts as mean outcome values and random slopes as direct predictor effects.
- Adds "marginal" type option to
estimate_grouplevel()function - Implements marginal estimation using
estimate_means()andestimate_slopes() - Updates documentation to describe the new experimental method
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| R/estimate_grouplevel.R | Implements the new marginal estimation method and updates function logic |
| man/estimate_grouplevel.Rd | Updates documentation to describe the new "marginal" type option |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new experimental marginal method to estimate_grouplevel(), which is an interesting approach for interpreting group-level parameters. The implementation looks like a good start. My review focuses on improving the robustness and maintainability of this new feature. The main points are the need for automated tests to ensure correctness, removing commented-out test code from the source, and addressing some of the limitations of the experimental implementation, such as by providing warnings for unsupported cases like factor-based random slopes. Overall, a promising addition.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot could you add tests |
|
I asked Gemini to add a test, not sure if that test is super good or not. But I saw in your "Quick examples" that the first example returns a different value than expected? library(modelbased)
model <- lme4::lmer(mpg ~ hp + (1 | carb), data = mtcars)
model <- lme4::lmer(mpg ~ hp + (1 + hp | carb), data = mtcars)
#> boundary (singular) fit: see help('isSingular')
model <- lme4::lmer(mpg ~ hp + (1 + hp | carb) + (1 | gear), data = mtcars)
#> boundary (singular) fit: see help('isSingular')
m1 <- estimate_grouplevel(model, type = "random")
m2 <- estimate_grouplevel(model, type = "total")
m3 <- estimate_grouplevel(model, type = "marginal")
cor.test(m1$Coefficient, m3$Coefficient) # r = 1
#>
#> Pearson's product-moment correlation
#>
#> data: m1$Coefficient and m3$Coefficient
#> t = 0.39984, df = 13, p-value = 0.6958
#> alternative hypothesis: true correlation is not equal to 0
#> 95 percent confidence interval:
#> -0.4261020 0.5892142
#> sample estimates:
#> cor
#> 0.1102191
cor.test(m2$Coefficient, m3$Coefficient) # r = 1
#>
#> Pearson's product-moment correlation
#>
#> data: m2$Coefficient and m3$Coefficient
#> t = 61.877, df = 13, p-value < 2.2e-16
#> alternative hypothesis: true correlation is not equal to 0
#> 95 percent confidence interval:
#> 0.9947589 0.9994535
#> sample estimates:
#> cor
#> 0.9983066Created on 2025-10-04 with reprex v2.1.1 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #574 +/- ##
==========================================
+ Coverage 90.63% 90.72% +0.09%
==========================================
Files 40 40
Lines 3364 3397 +33
==========================================
+ Hits 3049 3082 +33
Misses 315 315 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Should we probably also add |
@strengejacke yeah it's because we were correlating apples and potatoes
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Yes I think this makes sense |
This reverts commit ed32638.
@DominiqueMakowski this issue with `find_random_slope()` is a design choice, as the *name* refers to which random effects the slopes belong to (normal, zero-inflated, dispersion random etc.)
Co-authored-by: Dominique Makowski <dom.mak19@gmail.com>
Recently I was embroiled in discussions about how to interpret (i.e., the unit) of grouplevel estimates, which can become tricky for complex models. I suggested estimating group-level estimates directly using marginal means rather than raw ranefs. This actually would be a useful addition to estimate_grouplevel.
Currently only works on simple-ish cases, there are edgecases where it probably needs to be tested.
But it works, currently correlates perfectly r=1 with the other methods.