search

topipa / iwmm

iwmm: an R package for adaptive importance sampling
4 stars 2 forks source link

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

Closed n-kall closed 1 year ago

n-kall commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

n-kall commented 1 year ago

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 commented 1 year ago

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 :see_no_evil: I can take a look at both and pick relevant changes from both of them.

n-kall commented 1 year ago

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 commented 1 year ago

I merged https://github.com/topipa/iwmm/pull/3, so closing this now!