Fix Bayesian R2 computation for normal and Bernoulli#1880
Fix Bayesian R2 computation for normal and Bernoulli#1880florence-bockting wants to merge 6 commits intomasterfrom
Conversation
|
@avehtari can you check the current implementation?
This part is not yet implemented as I read it as a task for future implementations but I am not sure whether I misread this point - therefore I want to point this explicitly out here. |
|
@avehtari Can you please check the correctness of the computation. Thank you. |
|
|
||
| .bayes_R2_var_res_gaussian <- function(sigma, ...) { | ||
| sigma <- as.matrix(sigma) | ||
| matrixStats::rowMeans2(sigma^2) |
There was a problem hiding this comment.
I don't understand why there is rowMeans. The paper and the online appendix provided equations only for scalar sigma, and I think just taking mean for vector sigma in heteroscedastic case is not correct. Even if more correct approach would be too complicated to implement, at least this approximation should be documented.
There was a problem hiding this comment.
Thank you for pointing this out. I decided now for the following approach: check whether sigma is heteroscedastic. If true, a warning is provided and R2 computation falls back to residual-based R2.
There was a problem hiding this comment.
After discussing this point again, we (Aki, Florence) decided to use also for the heteroscedastic case the mean of sigma as an approximation as this is consistent with the discussion provided in Tjur (2009). A corresponding note is added in the function documentation for the user as well as in the code as a comment for the developer.
To sum up, we use model-based variances for (homo- and heteroscedastic) Gaussian and Bernoulli models. Otherwise residual-based variances are used and the user warned.
paul-buerkner
left a comment
There was a problem hiding this comment.
Looks good overall. just a few minor comments
| } | ||
| R2[[i]] <- .bayes_R2(y, ypred) | ||
| family_name <- family(object, resp = resp[i])$family | ||
| if (family_name %in% "gaussian") { |
There was a problem hiding this comment.
this pattern works for now. If we later on support the model-based approach for more than two families, we should provide separate functions per family, similar to how we do it with a lot of other post-processing funcftions.
There was a problem hiding this comment.
I refactored the code and outsourced the family-specific computations for better maintainability and easier future updating. Thank you for pointing this out.
|
Okay, we can leave it there in NEWS :-)
Am Mi., 29. Apr. 2026 um 17:08 Uhr schrieb Aki Vehtari <
***@***.***>:
… ***@***.**** commented on this pull request.
------------------------------
In NEWS.md
<#1880 (comment)>:
> @@ -19,6 +19,9 @@ Thanks to Sermet Pekin. (#1790)
### Bug Fixes
+* `bayes_R2` now uses model-based residual variances for Gaussian and Bernoulli
I think Bayes-R^2 is model based R^2, and the code was not implementing
model based R^2 and thus it was not implementing Bayes-R^2, but it was
named as bayes_R2, so if this is not a bug fix, it's a fix anyway.
—
Reply to this email directly, view it on GitHub
<#1880 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADCW2AEHHSEDMCQUCH7N7YT4YILG3AVCNFSM6AAAAACX5X27WWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DCOJYGIZDIMJVG4>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Description
This PR refers to Issue#1815.
Update
bayes_R2()to use model-based residual variance for supported families (Gaussian and Bernoulli), while preserving fallback behavior (i.e., residual-based R2) for unsupported families.Note, changes in this PR might be breaking as the output of the plot might change. However, the change in computation is rather considered as a "bug" and this PR as a fix. For safety reasons I run a reverse dependency check for this PR which indicated no issues with dependent packages.
Changes
bayes_R2.brmsfit()to compute R2 via model-based residual variance for:E[sigma^2]per draw)E[pi * (1 - pi)]per draw).bayes_R2_res().bayes_R2_model().bayes_R2_var_res_gaussian().bayes_R2_var_res_bernoulli()TODO
NEWS.md