biodiverse / ubms

Fit models to data from unmarked animals using Stan. Uses a similar interface to the R package 'unmarked', while providing the advantages of Bayesian inference and allowing estimation of random effects.
https://hmecology.github.io/ubms/
GNU General Public License v3.0
35 stars 8 forks source link

SD in predict() is NA if models are run with save_warmup = FALSE (issue in nsamples) #1

Closed jniedballa closed 4 years ago

jniedballa commented 4 years ago

Hi Ken, first of all, this is a fabulous package and overall works very well already. Really exciting. Thank you very much for your efforts.

Here's a little problem I encountered. I ran a big model and wished to save disk space by doing:

stan_occu(..., save_warmup = FALSE)

When I tried to use predict(), SD was all NA. I get values for SD though when I don't set save_warmup = FALSE. It turned out that nsamples() (called by predict) returns 0 because:

sum(object@stanfit@sim$n_save - object@stanfit@sim$warmup) with n_save = 2500 2500 2500 and warmup = 2500

I didn't follow the issue further (how exactly it impacts sim_lp etc) but it seems to be the cause of SD being NA in the prediction. Best regards, Jürgen

jniedballa commented 4 years ago

It might actually be as simple as: object@stanfit@sim$n_save - object@stanfit@sim$warmup2

In my example with save_warmup = FALSE I have: warmup = 2500 and warmup2 = 0 0 0

So using warmup2 instead of warmup may work. But I am not familiar enough with Stan to know exactly what the differences between warmup and warmup2 actually are. Cheers, Jürgen

kenkellner commented 4 years ago

I think you nailed it exactly, thanks. I fixed this here: aea7a57. Works now in my tests, let me know if it still doesn't for you. In general I haven't yet extensively tested passing arguments on to stan. I bet there are some other issues lurking there.

jniedballa commented 4 years ago

Thank you, predict now works as expected. On a side not and unrelated, after the update the package assumes a slot @transition in the submodels which previous versions didn't seem to create. Therefore, when trying to work with models created in previous versions, an error is produced:

Error in .local(object, ...) : 
  no slot of name "transition" for this object of class "ubmsSubmodel"

So I had to create that slot manually to work on models created with previous versions. Not a big issue, just want to mention it for the sake of backward compatibility.

In case anyone reading this is facing the same issue, this is the code I used to fix it (ubms_fit being the output of stan_occu()):

ubms_fit@submodels@submodels$det@transition   <- FALSE
ubms_fit@submodels@submodels$state@transition <- FALSE
kenkellner commented 4 years ago

Good catch. Now that I've incorporated a dynamic model in the package I should be done making changes to the basic data object structures. I'll try to build in better backwards compatibility if it becomes necessary.

kenkellner commented 4 years ago

The @transition backwards compatibility issue should be fixed now, in 0fe12380, which implements a child class for transition-type parameters instead.