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

Replace loo::psis with posterior::pareto_khat (when available) #2

Closed
n-kall opened this issue Apr 4, 2023 · 6 comments
Closed

Replace loo::psis with posterior::pareto_khat (when available) #2

n-kall opened this issue Apr 4, 2023 · 6 comments

Comments

@n-kall
Copy link
Collaborator

n-kall commented Apr 4, 2023

Currently loo::psis is used to calculate the pareto-k diagnostics, but there is work to have a pareto_khat diagnostic function in posterior. Using this instead would remove the dependency on loo and allow for loo to suggest iwmm (as discussed here).

I can make a branch implementing this change and create a PR once the corresponding functions are merged into posterior

@topipa
Copy link
Owner

topipa commented Jun 23, 2023

I made a draft of this change in the branch https://github.com/topipa/iwmm/tree/use-posterior-k. Seems to work locally with the current posterior package branch https://github.com/n-kall/posterior/tree/pareto_k (Pull request here) .

@n-kall
Copy link
Collaborator Author

n-kall commented Jun 23, 2023

Nice! There's also https://github.com/n-kall/iwmm/tree/posterior-pareto-k in case there is some detail helpful there

@n-kall
Copy link
Collaborator Author

n-kall commented Aug 31, 2023

The PR for posterior implementing pareto-k diagnostics is now merged! So this can go ahead, but there might have been some changes since this draft

@topipa
Copy link
Owner

topipa commented Aug 31, 2023

That's great! When I made the draft branch, I did not realize you had already done similar changes in your fork. So my changes are mostly duplicate of yours 🙈 I can take a look at both and pick relevant changes from both of them.

@n-kall
Copy link
Collaborator Author

n-kall commented Sep 6, 2023

No worries, I forgot to mention my progress earlier.

I can take a look at both and pick relevant changes from both of them.

This sounds perfect! Let me know if you have any questions on the pareto-k functionality in posterior or my attempt in the fork

@topipa
Copy link
Owner

topipa commented Sep 6, 2023

I merged #3, so closing this now!

@topipa topipa closed this as completed Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants