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

Randomised -> non-randomised PIT for count values #913

Closed
sbfnk opened this issue Sep 18, 2024 · 3 comments · Fixed by #949
Closed

Randomised -> non-randomised PIT for count values #913

sbfnk opened this issue Sep 18, 2024 · 3 comments · Fixed by #949
Assignees
Labels
high-priority Should be addressed soon

Comments

@sbfnk
Copy link
Contributor

sbfnk commented Sep 18, 2024

We're currently using the randomised version of PIT for count values - however Czado et al. (2009) recommend a non-randomised version, plotting

image

I can't see a reason why we wouldn't use that version instead.

This could be addressed when addressing #359

@nikosbosse
Copy link
Contributor

Love it!

@sbfnk sbfnk mentioned this issue Sep 19, 2024
9 tasks
@nikosbosse
Copy link
Contributor

Adding the following comments from @sbfnk from #916:

There were two broader questions raised by this work:

  • the way get_pit and plot_pit interact makes addressing Randomised -> non-randomised PIT for count values #913 impossible - the nonrandomised method requires the bins to be known at the time the PIT is calculated
  • the way quantiles are plotted when the breaks don't coincide with the quantiles levels in the data doesn't really makes sense (only the ones that coincide are plotted). It might be nice here to make a (e.g. linear, optional) interpolation of the CDF, in which case we could plot a PIT with arbitrary breaks

Both of these cases could be solved if the get_pit() method is changed to take the bins/breaks as an argument, all calculations/approximations are moved there, and plot_pit just plots the resulting geom_col (which may or may not even be needed as a separate function - e.g. get_pit could have a plot argument or else it could just be left to the user). I would suggest to make this change (in a separate issue/PR combo).

@nikosbosse
Copy link
Contributor

I really like these suggestions and think ideally we should address them before the next CRAN release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high-priority Should be addressed soon
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants