Accept cmdstanvb, draws_array or draws_matrix as fitted_params #390
Conversation
used draws_to_csv in process_fitted_params added tests
Codecov Report
@@ Coverage Diff @@
## master #390 +/- ##
==========================================
- Coverage 88.22% 86.35% -1.87%
==========================================
Files 12 12
Lines 2683 2704 +21
==========================================
- Hits 2367 2335 -32
- Misses 316 369 +53
Continue to review full report at Codecov.
|
R/data.R
Outdated
| sampler_diagnostic_names <- c("accept_stat__", "stepsize__", "treedepth__", "n_leapfrog__", "divergent__", "energy__") | ||
|
|
||
| n <- posterior::niterations(draws) | ||
| n_chains <- posterior::nchains(draws) | ||
| if (is.null(sampler_diagnostics)) { | ||
| # create dummy sampler diagnostics due to CmdStan requirement for all columns in GQ | ||
| sampler_diagnostics <- rep(0, n * length(sampler_diagnostic_names) * n_chains) | ||
| dim(sampler_diagnostics) <- c(n, n_chains, length(sampler_diagnostic_names)) | ||
| sampler_diagnostics <- posterior::as_draws_array(sampler_diagnostics) | ||
| posterior::variables(sampler_diagnostics) <- sampler_diagnostic_names | ||
| } | ||
|
|
||
| # the columns must be in order "lp__, sampler_diagnostics, parameters" | ||
| variables <- posterior::variables(draws) | ||
| # create a dummy lp__ column if it does not exist | ||
| if ("lp__" %in% variables) { | ||
| lp__ <- NULL | ||
| } else { | ||
| lp__ <- rep(0, n * n_chains) | ||
| dim(lp__) <- c(n, n_chains, 1) | ||
| lp__ <- posterior::as_draws_array(lp__) | ||
| posterior::variables(lp__) <- "lp__" | ||
| } |
There was a problem hiding this comment.
This looks good, but I think there's a slightly nicer way code it using the posterior::draws_array() constructor. I can do that in a few minutes and push it.
There was a problem hiding this comment.
That would be fantastic! Thanks!
There was a problem hiding this comment.
Just pushed it. Let me know if you like it. If not we can revert because what you had definitely also worked.
|
I also just update the doc for the fitted_params argument. |
|
Oh right, thanks! |
|
I didn't test this on any of my own models but the tests you wrote seem like they cover all the cases, so if you like the coding changes I made to |
|
Everything looks much nicer! Thanks. Will merge when tests pass. |
|
Looks like all checks are passing so I'm going to go ahead and merge this. |
Summary
Fixes #387
Copyright and Licensing
Please list the copyright holder for the work you are submitting
(this will be you or your assignee, such as a university or company):
Rok Češnovar, Uni. of Ljubljana
By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses: