search

stan-dev / shinystan

shinystan R package and ShinyStan GUI
https://mc-stan.org/shinystan
GNU General Public License v3.0
197 stars 51 forks source link

Default for "max_treedepth"/"max_td" (11) incorrect? #183

Closed fweber144 closed 4 years ago

fweber144 commented 4 years ago

The output of shinystan:::.rstan_max_treedepth() (the maximum tree depth) defaults to 11, even though Stan's default is 10. A value of 11 would be correct if iterations were checked for hitting the maximum tree depth with "tree depth == max_td - 1", but it seems like downstream code in shinystan uses the check "tree depth == max_td". Besides, shinystan:::.rstan_max_treedepth() sets max_td to x@stan_args[[1L]][["control"]]$max_treedepth if existing (with x being a stanfit object), which is inconsistent with the default of 11.

jgabry commented 4 years ago

Yeah that's weird, thanks for reporting this! @VeenDuco we should definitely fix this for version 3, but it's probably worth fixing for version 2.x also.

VeenDuco commented 4 years ago

Sounds good to fix this in both versions. I'll try to get to this next week. However, I'm starting the new job on monday and I've had to clean out my previous (work) laptop and I'll have to setup my new computer again. That could delay things a bit. I'll keep you updated.

VeenDuco commented 4 years ago

Fixed in both version. It is just an assignment for if the max_td is unknown from the object that we get. It makes more sense to default in to 10 though.

jgabry commented 4 years ago

Thanks @VeenDuco, And congrats on the new job!