topipa / iwmm

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

Brmsfit method #12

Closed n-kall closed 6 months ago

n-kall commented 8 months ago

updates to brmsfit method, passes the tests for me. Might be still some work to do to clean it up

n-kall commented 8 months ago

Ah, yes this happens to me too. Seems to be the print method is failing. The object contains the new fit but must be missing something

n-kall commented 7 months ago

I now added a function to create dummy sampler_params so the brmsfit object no longer gives an error on print.

The issue is that the sampler_params are stored for each chain, and moment matching merges all chains. Also, as the draws changed, I am not sure that the NUTS parameters are relevant any more anyway. Now in the modified brmsfit, all sampler_params are just numeric(0)

topipa commented 6 months ago

I now added a function to create dummy sampler_params so the brmsfit object no longer gives an error on print.

The issue is that the sampler_params are stored for each chain, and moment matching merges all chains. Also, as the draws changed, I am not sure that the NUTS parameters are relevant any more anyway. Now in the modified brmsfit, all sampler_params are just numeric(0)

Oh that was the missing piece. Thank you for taking care of it! The sampler parameters might be useful for something even after moment matching, but we can set them to 0 for now :+1: