-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
Add R2 #205
Add R2 #205
Conversation
Thanks for the PR, will try to review soon! @avehtari what do you think about adding this? |
@sims1253 Can you pull in the latest changes from the master branch into your PR (I updated the GitHub actions workflow files on the master branch) so that it runs the tests again? Thanks |
Forgot to push the reference values for the tests >.> Lost them in all the styler changes I didn't want to add to this pr. |
Codecov Report
@@ Coverage Diff @@
## master #205 +/- ##
==========================================
- Coverage 93.48% 93.43% -0.05%
==========================================
Files 29 30 +1
Lines 2715 2741 +26
==========================================
+ Hits 2538 2561 +23
- Misses 177 180 +3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
I'm in favor of adding R2. We need to be careful that all these additional utility/loss functions (see #202 and #203) have the same behavior for the user. Based on a quick look, the SE is not computed correctly, as it's ignoring the uncertainty in ss_y. If I remember correctly, we did go through the correct derivation with @LeeviLindgren in summer? |
@avehtari We derived the first-order Taylor approximation of the variance. What we came up with is |
Thanks for the feedback. If my memory serves me right, we defined R2 in a slightly different way than what you used in your case study to get the pointwise entries for psis-loo and the SE. But this was a year ago so I need to go check my notes to see what I thought back then and won't have time this week. |
@sims1253, I'm just checking whether you have had time to check your notes? |
@sims1253 any update on this? |
I assumed that I could use the same formula as in This is from my notes: We integrate/mean over the posterior samples (and ignore the sum and 1/N until later) Which we can factorize into and reform into If the s's in the I am missing a step here in my notes for handling all those 1's :( Probably obvious if one can handle integrals better than I can. Adding back the sum and 1/N gives us which is defined pointwise and integrates over posterior samples. So we can apply psis weights and have the benefit of having a pointwise result for SE estimation. (I am sorry but I can't find the reason for these not to render properly) Moving on to R2, we use the following definition of R2 As there are not posterior samples in the denominator the integral form looks like this: $$\int R^2 dp(\hat{y}) = 1 - \frac{\sum_{n=1}^N \int (y_n - \hat{y}_n)^2 dp(\hat{y}n)}{\sum{n=1}^N (y_n - \bar{y})^2}$$ $$= \sum_{n = 1}^N ( \frac{1}{N} - \frac{\int (y_n - \hat{y}n)^2}{\sum{k = 1}^N (y_k - \bar{y})^2})$$ For psis we only have to weight the nominator and again we have a pointwise result. |
It seems github can't handle sum in frac. I was able to preview them elsewhere. It seems like you are confusing things, as the PSIS part is not an issue, but the part that we don't know the distribution of the future data, which makes the nominator and denominator dependent. @LeeviLindgren comment above gives the correct formula for Var(R2_loo). |
Ah I think I understand the problem now, thanks for sticking with me. Will try to update this PR over the holidays. |
Closing this for now so it doesn't clutter your PR list. |
@LeeviLindgren sorry for digging this out; it's because of stan-dev/projpred#496. Do I understand
in #205 (comment) correctly that you applied a bivariate delta method to the function |
This is based on the code I wrote for bayesim. Related issue #201