Add ppc_loo_pit_ecdf

[TeemuSailynoja]

Made an implementation of the ppc_pit_ecdf() for LOO-PITs. This is mostly a convenience function, as one could already pass LOO-PITs to the ppc_pit_ecdf in its pit argument. Now works directly with for example brms::pp_check(brms_fit, "loo_pit_ecdf")

I have two questions:

1. ppc_loo_pit_ecdf() uses a prob argument. ppc_loo_pit_intervals already imports the args-prob-prob_outer template, which has a different doc string for prob. This template is used outside the ppc_loo functions, so I don´t think adding text specific to this new function there is the best solution. How should this be handled?
2. I made ppc_loo_pit_ecdf have a slightly different signature to ppc_pit_ecdf. Namely, instead of K, this new function uses a more informative eval_points, and instead of the boolean interpolate_adj, now I have interval_method = c("interpolate", "optimize"). Should I hold on to this change and have the functions share their signature? I think the new one is more clear and ppc_pit_ecdf should also move towards that eventually.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 95.79832% with 5 lines in your changes missing coverage. Please review.

Project coverage is 98.60%. Comparing base (4d40ba6) to head (f8fab2f).

Files with missing lines	Patch %	Lines
R/ppc-loo.R	95.79%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #345      +/-   ##
==========================================
- Coverage   98.66%   98.60%   -0.06%     
==========================================
  Files          35       35              
  Lines        5466     5533      +67     
==========================================
+ Hits         5393     5456      +63     
- Misses         73       77       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jgabry]

1. ppc_loo_pit_ecdf() uses a prob argument. ppc_loo_pit_intervals already imports the args-prob-prob_outer template, which has a different doc string for prob. This template is used outside the ppc_loo functions, so I don´t think adding text specific to this new function there is the best solution. How should this be handled?

Maybe we don't use the args-prob-prob_outer template for ppc_loo_pit_intervals and instead you can do something like this?

@param prob For `ppc_loo_pit_intervals()` `prob` is used for X. For `ppc_loo_pit_ecdf()` `prob` is used for Y.`

where X just basically says what's in the template. Not optimal to copy what's in the template I guess, but not a big deal.

2. I made ppc_loo_pit_ecdf have a slightly different signature to ppc_pit_ecdf. Namely, instead of K, this new function uses a more informative eval_points, and instead of the boolean interpolate_adj, now I have interval_method = c("interpolate", "optimize"). Should I hold on to this change and have the functions share their signature? I think the new one is more clear and ppc_pit_ecdf should also move towards that eventually.

It would be nice to have the same signature, but I agree with you that your new one is more clear. Since the new one is better, it's ok with me to have a different signature for now until we can also update the one for ppc_pit_ecdf.

[TeemuSailynoja]

I also unified the arguments for now, to keep the code more simple.
I suggest that, when I work on the documentation and possibly need to rearrange how the PPC function pages are grouped, I will add the clearer argument names and add a deprecation warning to the old ones.

[TeemuSailynoja]

Also, with this rearranging, the template could come back to use with ppc_loo_pit_intervals().

It would probably make sense to have ppc_(loo)_pit_ecdf(), ppc_loo_pit_qq(), and ppc_loo_pit_overlay() share a documentation page, and move ppc_loo_intervals(), and ppc_loo_ribbon() join their LOOless counterparts on the PPC intervals documentation page.

[jgabry]

I also unified the arguments for now, to keep the code more simple.
I suggest that, when I work on the documentation and possibly need to rearrange how the PPC function pages are grouped, I will add the clearer argument names and add a deprecation warning to the old ones.

Sounds good, I like that plan.

[jgabry]

Also, with this rearranging, the template could come back to use with ppc_loo_pit_intervals().

It would probably make sense to have ppc_(loo)_pit_ecdf(), ppc_loo_pit_qq(), and ppc_loo_pit_overlay() share a documentation page, and move ppc_loo_intervals(), and ppc_loo_ribbon() join their LOOless counterparts on the PPC intervals documentation page.

That also sounds good. When I first included those LOO-based plots I thought it made sense to put them all together. But I think you're right that this reorganization makes sense at this point. I think we'll just want to link between the documentation pages, pointing out that there are other LOO-based plots in various locations.

[jgabry]

The code looks great. I will try to make some time later today to actually try using the function. One minor request for now: can you mention this function in the "Plot Descriptions" section of the documentation?

[jgabry]

I think for future compatibility with ggplot2 we probably need to do what was done in https://github.com/stan-dev/bayesplot/pull/337/files#diff-d31e64955d39a8924be5d979cb21c5e96514987aeff9ca8cf2af067e98f3b156 with checking for the get_labs() function and using it if found.

[jgabry]

(I just noticed that I haven't done this for the tests for ppc_loo_pit_qq either. If you want you can do it with this PR but I can also do it separately since it's not related to your PR)

[TeemuSailynoja]

I can also add the part for ppc_loo_pit_qq.

[jgabry]

The function works great. I made one small comment on the tests and then there's mentioning the function in the "Plot Descriptions" section of the doc, but otherwise I think this is ready without any other changes.

One strange thing is that two of the visual tests are failing for me locally but apparently not on Github Actions. But they're not your tests for the new function, they're ppc-loo-pit-overlay-default.svg and ppc-loo-pit-qq-default.svg. Does everything pass for you locally?

[TeemuSailynoja]

The function works great. I made one small comment on the tests and then there's mentioning the function in the "Plot Descriptions" section of the doc, but otherwise I think this is ready without any other changes.

One strange thing is that two of the visual tests are failing for me locally but apparently not on Github Actions. But they're not your tests for the new function, they're ppc-loo-pit-overlay-default.svg and ppc-loo-pit-qq-default.svg. Does everything pass for you locally?

For me, all tests are passing. I have now added the forward compatibility for the tests of ppc_loo_pit_ecdf, and ppc_loo_pit_qq and included a short description of the function under "Function Descriptions".