Expose new stan args

[venpopov]

Submission Checklist

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

Summary

Expose the new arguments save_metric and save_cmdstan_config in Stan 2.34 as arguments in cmdstanr methods. Closes #931

• the save_metric is now an argument to sample()
• save_cmdstan_config is an argument for all sampling methods
• both of these are set TRUE by default (as this is also the plan in CmdStanPy)
• new methods metric_files(), config_files(), save_metric_files() and save_config_files() work similarly to the existing methods output_files() and save_output_files()

Now by default in addition to the output files for each chain, "*_metric.json" and "*_config.json" files are created alongside.

---

This PR only deals with the above. As I mentioned in #931, now that these are available, it would be good to read the metadata information directly from them at the end of sampling, instead of greping the metadata from the output files, to finalize the fit object faster. I can handle that next.

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):
Vencislav Popov

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/)

[venpopov]

When using the new functions I noticed that cmdstan gives an error if running a model with adapt_engaged=FALSE and save_metric=TRUE. So I also added a test showing the failure and a check to turn off save-metric if adapt_engaged is FALSE

[jgabry]

Thank you! Will try to review this soon.

[jgabry]

As I mentioned in #931, now that these are available, it would be good to read the metadata information directly from them at the end of sampling, instead of greping the metadata from the output files, to finalize the fit object faster. I can handle that next.

This sounds great.

[venpopov]

Strange, the cmd check passed locally. Should I try to fix the issues or do you want to review the general approach first?

[jgabry]

I think the general approach is good, so if you want to take a look at the R cmd check failures now (at first glance it seems to be just a couple of small things) I think that's fine.

Possibly later today but maybe later this week I'll have a chance to make a few comments on various details in the PR, but overall looks good!

[venpopov]

Ok, I'll see to fixing the checks. As for the reading the metadata from the new files, I have noted a few questions to discuss first, but I'll probably have to post about it next week

[jgabry]

Ok, I'll see to fixing the checks.

Ok thanks. Otherwise this looks really good! I haven't had a chance to really test it yet, but I don't see any issues with the changes to the code.

As for the reading the metadata from the new files, I have noted a few questions to discuss first, but I'll probably have to post about it next week

Sounds good, thank you.

[venpopov]

When using the new functions I noticed that cmdstan gives an error if running a model with adapt_engaged=FALSE and save_metric=TRUE. So I also added a test showing the failure and a check to turn off save-metric if adapt_engaged is FALSE

that was the main culprit - the tests ran clean on the origin PR, and I did not run them again after this small change. I fixed and now everything runs clean on my machine, so hopefully the remote tests will pass as well.

[venpopov]

All the tests pass, except for WSL, which gives an error not being able to find the metric files. I'm not sure what to do about that - I don't have a WSL setup, so I don't know what the problem is or how to debug it.

[jgabry]

All the tests pass, except for WSL, which gives an error not being able to find the metric files. I'm not sure what to do about that - I don't have a WSL setup, so I don't know what the problem is or how to debug it.

Thanks!

@andrjohns would you be able take a peek at the WSL issue?

[venpopov]

Just bumping this up since it seems just that small issue with the WSL backend test is left. I could check the WSL tests in principle, but I would have to install a virtual windows machine and set up everything just for this. It would be great if someone who already has access to WSL backend could check what the problem is ;)

[jgabry]

Hoping @andrjohns can take a look at this since I don't know much about WSL. @andrjohns if you're too busy at the moment let me know and I can try to find someone else who knows WSL to take a look.

[andrjohns]

Sorry for the delay, I'll have time to fix up the WSL failures and review on Friday

[andrjohns]

Some minor requests and then this should be good to go. Thanks!

[andrjohns]

Can you undo these whitespace changes? There are a few throughout the diff

[andrjohns]

Suggested change

	save_metric = TRUE,
	save_cmdstan_config = TRUE,
	save_metric = if (cmdstan_version() > "2.34.0") TRUE else NULL,
	save_cmdstan_config = if (cmdstan_version() > "2.34.0") TRUE else NULL,

Then the function arguments below would just use the save_metric and save_cmdstan_config values directly

[andrjohns]

And the same for the other methods as well (laplace, optimize, etc.)

[jgabry]

good catch, missed that when I was looking at it

[andrjohns]

@venpopov I've gone ahead implemented my requests, let me know if you have any other changes or updates you want to make, otherwise I'll merge this

Thanks again for putting this together!

[venpopov]

Thanks! I've been on vacation without a computer and was planning to get to it next week. But if you are happy with the current state, feel free to merge 👍