n-kall / priorsense

priorsense: an R package for prior diagnostics and sensitivity
https://n-kall.github.io/priorsense/
GNU General Public License v3.0
54 stars 5 forks source link

Collection of fixes and enhancements for branch `separate_scaling` #20

Closed fweber144 closed 1 year ago

fweber144 commented 1 year ago

This is a more or less loose collection of fixes and enhancements on top of branch separate_scaling that I found while working with priorsense, especially in a predictive setting.

The commit messages should provide details where necessary. If you need more information, just let me know.

I have also added myself as a contributor. If you don't want this, simply revert the corresponding commit.

I have not run R CMD check or the unit tests at all. Please tell me if I should do so.

n-kall commented 1 year ago

Thanks @fweber144 for all of this! I'll take a closer look but it's likely good to merge as is

fweber144 commented 1 year ago

For the prediction part in powerscale_sequence.priorsense_data(), I'm now realizing that my changes might be unnecessarily complicated. The approach in powerscale.priorsense_data() and powerscale_gradients.priorsense_data(), namely to initially assign only base_draws <- x$draws, then appending the predictions, and then to call posterior::subset_draws() would probably be more straightforward and would avoid the extra definition of variable_base. But I'm not sure if that approach is really transferable to powerscale_sequence.priorsense_data().

n-kall commented 1 year ago

These changes look all good to me! I'll merge now and if we want to change the prediction part as you said, we can do that with another PR

fweber144 commented 1 year ago

Thanks!