$unconstrain_draws() returns draws format

[andrjohns]

Submission Checklist

• Run unit tests
• Declare copyright holder and agree to license (see below)

Summary

Adds a format argument to the unconstrain_draws() method to return the draws in a draws format

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):
Andrew Johnson

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses:

• Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
• Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

[andrjohns]

This would technically be a breaking change, since it changes the default return from a list of lists (list per chain) to a single draws object. But I think it's the way I should have implemented it from the start, so it's worth breaking.

@avehtari would you be able to try out this branch and check that it works for your use-cases?

@n-kall priorsense is the main user of $unconstrain_draws() at the moment (that I'm aware of), does this all look good to you?

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2bec769) 88.35% compared to head (552777b) 86.73%.

❗ Current head 552777b differs from pull request most recent head a43a178. Consider uploading reports for the commit a43a178 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #886      +/-   ##
==========================================
- Coverage   88.35%   86.73%   -1.63%     
==========================================
  Files          12       12              
  Lines        4527     4530       +3     
==========================================
- Hits         4000     3929      -71     
- Misses        527      601      +74     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jgabry]

Thanks!

This would technically be a breaking change, since it changes the default return from a list of lists (list per chain) to a single draws object. But I think it's the way I should have implemented it from the start, so it's worth breaking.

Yeah I think this is worth it.

[n-kall]

Sorry for the delay, just got some time to try this out. Looks great! It should make it easier to unconstrain -> transform -> constrain for moment matching. Thanks @andrjohns !

[avehtari]

I can confirm that the following works now

summarise_draws(fit1$unconstrain_draws(draws=as_draws(pth1)), variance)

Note that I had to again use fit1 from MCMC sampling, as pth1$unconstrain_draws() still had the problem which was supposed to be fixed, see #872

> model1 <- cmdstan_model(stan_file = root("Birthdays", "gpbf1.stan"),
                        include_paths = root("Birthdays"),
                        compile_model_methods=TRUE, force_recompile=TRUE)
> pth1 <- model1$pathfinder(data = standata1, init=0.1, num_paths=10, single_path_draws=40,
                          history_size=100, max_lbfgs_iters=100)
> summarise_draws(pth1$unconstrain_draws(draws=as_draws(pth1)), variance)
Error: The method has not been compiled, please call `init_model_methods()` first
> pth1$init_model_methods()
Error: Model methods cannot be used with a pre-compiled Stan executable, the model must be compiled again

[n-kall]

@andrjohns Do you think it would make sense to also have a constrain_draws method that would convert the output of unconstrain_draws (potentially transformed) back to the constrained space? With this PR, currently one needs to do something like apply(fit$unconstrain_draws(format = "draws_matrix"), 1, fit$constrain_variables), and then clean up the resulting list.

One other thing: the default draws format is currently draws_df, but I think draws_array is usually the default format. e.g. in $draws(). Perhaps this should be changed to format = getOption("cmdstanr_draws_format", "draws_array") to match?

[andrjohns]

I can confirm that the following works now

summarise_draws(fit1$unconstrain_draws(draws=as_draws(pth1)), variance)

Note that I had to again use fit1 from MCMC sampling, as pth1$unconstrain_draws() still had the problem which was supposed to be fixed, see #872

> model1 <- cmdstan_model(stan_file = root("Birthdays", "gpbf1.stan"),
                        include_paths = root("Birthdays"),
                        compile_model_methods=TRUE, force_recompile=TRUE)
> pth1 <- model1$pathfinder(data = standata1, init=0.1, num_paths=10, single_path_draws=40,
                          history_size=100, max_lbfgs_iters=100)
> summarise_draws(pth1$unconstrain_draws(draws=as_draws(pth1)), variance)
Error: The method has not been compiled, please call `init_model_methods()` first
> pth1$init_model_methods()
Error: Model methods cannot be used with a pre-compiled Stan executable, the model must be compiled again

Great, thanks @avehtari! The pathfinder issue was just because this branch hadn't had the new changes to master merged in yet, so the fix wasn't present.

@andrjohns Do you think it would make sense to also have a constrain_draws method that would convert the output of unconstrain_draws (potentially transformed) back to the constrained space? With this PR, currently one needs to do something like apply(fit$unconstrain_draws(format = "draws_matrix"), 1, fit$constrain_variables), and then clean up the resulting list.

One other thing: the default draws format is currently draws_df, but I think draws_array is usually the default format. e.g. in $draws(). Perhaps this should be changed to format = getOption("cmdstanr_draws_format", "draws_array") to match?

Good call, thanks @n-kall! I've updated the draws format to match, and I'll get started on a complementary constrain_draws() method