174 cmdstanr sample args

[kaitejohnson]

Adding all non-deprecated arguments to cmdstanr::sample() to be configurable via get_mcmc_options(). Closes #174.

Also turn off messages for testing so that we are not getting CI warnings about ESS being capped. Note this doesn't seem to be working just by setting show_messages = FALSE, not seeing anything in global options either...
https://mc-stan.org/cmdstanr/reference/cmdstanr_global_options.html

@athowes @seabbs Do you have any suggestions from your experience testing large stan models? Suppressing the output in CI can be a separate issue #176. Guess I can always just use expect_warning()

[athowes]

What are the challenges here with testing it sorry? Some of the parameters have high Rhat because there are so many parameters?

[dylanhmorris]

Need to discuss the way in which defaults are being specified here. I do not think expecting the user to call a function that just builds a list is ideal.

Instead, I would suggest allowing the user just to provide the list directly. If you want to be able to fall back on defaults, I would do something like the following

wwinference <- function(ww_data,
                        count_data,
                        forecast_date = NULL,
                        calibration_time = 90,
                        forecast_horizon = 28,
                        model_spec = get_model_spec(),
                        fit_opts = list(),
                        generate_initial_values = TRUE,
                        initial_values_seed = NULL,
                        compiled_model = compile_model()) {
   # ... other code here
    fit_opts_use <- get_default_fit_opts()
    fit_opts_use[names(fit_opts)] <- fit_opts  # this overwrites all and only the values the user sets in `fit_opts`
    # maybe check for invalid options here?
   # .. more code here
}

If you really want to enforce argument names you can check the final list names correspond to valid $sample() arguments

[kaitejohnson]

Need to discuss the way in which defaults are being specified here. I do not think expecting the user to call a function that just builds a list is ideal.

Instead, I would suggest allowing the user just to provide the list directly. If you want to be able to fall back on defaults, I would do something like the following

wwinference <- function(ww_data,
                        count_data,
                        forecast_date = NULL,
                        calibration_time = 90,
                        forecast_horizon = 28,
                        model_spec = get_model_spec(),
                        fit_opts = list(),
                        generate_initial_values = TRUE,
                        initial_values_seed = NULL,
                        compiled_model = compile_model()) {
   # ... other code here
    fit_opts_use <- get_default_fit_opts()
    fit_opts_use[names(fit_opts)] <- fit_opts  # this overwrites all and only the values the user sets in `fit_opts`
    # maybe check for invalid options here?
   # .. more code here
}

If you really want to enforce argument names you can check the final list names correspond to valid $sample() arguments

In this proposed syntax, what you are suggesting basically is not specifying the default to be in the wwinference() function, but instead within it (e.g. replace what you wrote as get_default_fit_opts() with the current get_mcmc_options().

This seems pretty similar to me, I don't think I am following how it is different other than the user calling a function to build a list vs it being done internally within the wwinference() function. Maybe missing something.

[dylanhmorris]

As things are currently specified, the user is expected to configure options by doing:

fit_opts = get_mcmc_options(# my choices of options go here, as keyword arguments
)

They could also pass a list:

fit_opts = list(# my choices of options go here, as list entries
)

but in that case they wouldn't get the wwinference defaults for any unspecified options. Instead, sample() would fall back on its own defaults.

What I am suggesting is that a user shouldn't have to call a custom function of ours to override some but not all of our defaults, and should instead be able to pass a list.

[kaitejohnson]

As things are currently specified, the user is expected to configure options by doing:

fit_opts = get_mcmc_options(# my choices of options go here, as keyword arguments
)

They could also pass a list:

fit_opts = list(# my choices of options go here, as list entries
)

but in that case they wouldn't get the wwinference defaults for any unspecified options. Instead, sample() would fall back on its own defaults.

What I am suggesting is that a user shouldn't have to call a custom function of ours to override some but not all of our defaults, and should instead be able to pass a list.

I see, and we'd make sure that the elements in that list are only the available args to cmdstanr::sample()?

[dylanhmorris]

I see, and we'd make sure that the elements in that list are only the available args to cmdstanr::sample()?

Yes, though it might also be reasonable to defer to cmdstanr's own input validation there

[seabbs]

https://github.com/epinowcast/epinowcast/blob/12c24bf509704c4384902408f36dff86e73fef5b/tests/testthat/helper-functions.R#L74

[dylanhmorris]

@kaitejohnson is this ready for review?

[dylanhmorris]

This is looking very good, thanks @kaitejohnson. A few final things, and then should be ready to go.

[kaitejohnson]

Need to have chains in get_mcmc_options() even though its the default bc wwinference() has to know how many chains to run

[github-actions]

Thank you for your contribution, @kaitejohnson 🚀! Your page is ready to preview here

[dylanhmorris]

Thanks, @kaitejohnson. A few final things.

[dylanhmorris]

LGTM