Cmdstanr: Passing metric as a file or R object

[bbbales2]

In Cmdstan the metric argument determines the type of metric and then the metric_file argument provides the path.

Currently in cmdstanr metric can take a file path, but it doesn't then break the arguments out correctly to Cmdstan (it just sets cmdstan metric to a path and that gives an error).

I think we either break this out into the two arguments cmdstan uses (metric and metric_file) or something that makes sense from R (metric and metric_value), where metric_value is a vector/matrix that cmdstanr then writes to a file.

I'm gonna try to do the metric, metric_file thing.

[jgabry]

Thanks! I think it would be convenient for users if we also allow metric to be an R object, eg numeric vector. cmdstanr can write it to json and pass it to metric_file. What do you think?

[bbbales2]

cmdstanr can write it to json and pass it to metric_file. What do you think?

It would be more convenient. If you want to implement this, feel free to take over. Otherwise I'm just gonna do metric_file for now.

I think specifying metric_value is better than metric_file totally.

[bbbales2]

No wait. I don't know why I wouldn't just do metric_value right off the bat. It seems just as easy.

All I need to do is add some arguments to the model.R and args.R files.

My question is where would I create the temporary metric files and save references to them?

RunSet seems like the natural place cause that's where the diagnostic and output files are saved, but that is shared across MCMC and variational inference and optimization. This is specific to MCMC.

I guess I would do this in args?

[jgabry]

Thanks! Yeah I guess the file could just be created immediately when the SampleArgs object is initialized. And I think you can just use the write_stan_json function to write the json file.

[jgabry]

Also, I guess we should also allow metric_file since it's in CmdStan. Is that what you were thinking? Or do you think we should _only_ allow metric_value (in addition to metric)?

[bbbales2]

I think only metric_value. Or I guess the natural argument would be inv_metric since that's what you'd regularly specify in metric_file.

I think we skip the metric_file option. In the same way that in cmdstanr we pass in data as lists (instead of filenames), it makes sense to encapsulate this.

[bbbales2]

And by encapsulate this I mean actually hide this interface to cmdstan.

[jgabry]

I'm fine with that (although technically we still allow passing data as a file)

[jgabry]

I agree that inv_metric makes more sense as a name for this. But what about the existing metric argument that takes a string?

[jgabry]

Or maybe metric_inv, so at least it starts with metric

[bbbales2]

although technically we still allow passing data as a file

Man that's what I get for being confident when I didn't actually look it up lol.

What I'll target is:

1. If metric is a vector or a matrix and metric_file is NULL, write these to files and use them

2. If metric is a string, check if it's diag_e, dense_e, unit_e and handle metric_file appropriately.

3. If metric is a vector or a matrix and metric_file is not NULL, error

[jgabry]

Thanks, sounds good!

[bbbales2]

Alright. Plan changed.

What I did was add another argument 'inv_metric'.

1. The metric argument takes "dense_e", "diag_e", "unit_e"

2. The inv_metric argument takes a vector or a matrix (or a list of vectors or a list of matrices)

3. The metric_file argument takes a file name (or a list of file names) pointing at files that have metrics defined in them.

I decided not to stack up on the metric argument cause the actual thing we're passing in is the inverse metric. Confusing lol.

I think keeping inv_metric and metric_file separate is fine. I'd like to collapse them, but I think the way you'd collapse them is keeping inv_metric, and then using cmdstanr is kinda different from cmdstan.

If you specify both inv_metric and metric_file you get errors.

Questions:

1. Most of the code is in SampleArgs::initialize. What I do is write files and store them in metric_file. It feels like this code should go in validate. Does validate get called once? If so I'll move it.

2. I wasn't able to use args$csv_basename() or num_chains from SampleArgs which was slightly inconvenient. But this doesn't really belong in RunSet cause that's generic for all the different inferences.

3. Is there a way to quickly run only specific tests? Running all the tests is slow. I can test in little side scripts I've written, but running the actual package tests is really slow (it's also making debugging the tests hard). I could look this up but I demand someone hold my hand instead.

[jgabry]

That seems reasonable.

3. The metric_file argument takes a file name (or a list of file names) pointing at files that have metrics defined in them.

You mean inverse metrics? ;)

[bbbales2]

Lol dangit

[bbbales2]

I opened a pull req: https://github.com/stan-dev/cmdstanr/pull/64

[jgabry]

3. Is there a way to quickly run only specific tests? Running all the tests is slow. I can test in little side scripts I've written, but running the actual package tests is really slow (it's also making debugging the tests hard). I could look this up but I demand someone hold my hand instead.

Yes! For example devtools::test(filter = pattern), where pattern is a regular expression referring to names of test files. So, for example, to just run tests in test-model.R and test-fit.R you could run this (assuming you're in the package working directory, e.g. via the RStudio cmdstanr project):

devtools::test(filter = "fit|model")

[bbbales2]

Alright nice, that worked way faster. I can do this.

[jgabry]

Thanks for the PR!! I just made a bunch of (hopefully helpful) comments.