Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow structured format for unconstrained parameters #730

Open
andrjohns opened this issue Dec 19, 2022 · 8 comments
Open

Allow structured format for unconstrained parameters #730

andrjohns opened this issue Dec 19, 2022 · 8 comments
Labels
feature New feature or request

Comments

@andrjohns
Copy link
Collaborator

The current approach to providing unconstrained parameters as arguments (as inherited from rstan) requires that they are passed as a single numeric vector with all unconstrained parameter values concatenated.

After some API discussions with @n-kall, it was pointed out that this approach can easily lead to errors if any transformations or custom constructions of unconstrained parameters are performed - as users might accidentally provide parameter values in the wrong order.

Instead, we could allow unconstrained parameters to be passed as a named list like we do for constrained parameters, but with handling/checking for the different number of unconstrained values (e.g., for simplexes).

As a concrete example, for a model with parameters:

parameters {
  real x;
  simplex[4] y;
}

The unconstrained parameters would need to be passed as a four-element numeric vector:

unconstrained_vector <- c(1.2, 0.6, 1.6, -1.2)

This clearly allows for the easy mistake of accidentally providing the value of x as the last, rather than the first element (especially if the model is being iterated/revised).

I'm proposing we could accept the unconstrained parameters in the format:

unconstrained_list <- list(x = 1.2, y = c(0.6, 1.6, -1.2))

And then handle the ordering internally when mapping to a single vector of values.

Thoughts?

@andrjohns andrjohns added the feature New feature or request label Dec 19, 2022
@rok-cesnovar
Copy link
Member

Love it!

@jgabry
Copy link
Member

jgabry commented Dec 19, 2022

Great idea!

@mitzimorris
Copy link
Member

why is this needed in cmdstanr?
cmdstan only deals with inputs on the constrained scale.

@andrjohns
Copy link
Collaborator Author

The model methods functions exposed to R via c++ (log_prob, grad_log_prob, hessian) take their inputs on the unconstrained scale

@mitzimorris
Copy link
Member

this proposal only goes halfway towards validating the inputs - it provides names for the variables, but not structure. in particular, matrix and multi-dim array objects are flattened into a vector and there is no way of checking that the flattened structure is serialized correctly - i.e., column-major.

a meta-objection is that this feature is unecessary. the output from the diagnose method provides the correctly ordered vector of parameters on the unconstrained scale. is it really worth the cost of implementing, testing, and maintaining this feature - every feature added is maintenance debt going forwards.

@andrjohns
Copy link
Collaborator Author

this proposal only goes halfway towards validating the inputs - it provides names for the variables, but not structure

The structure is the point of this proposal. Similar to how providing initial values or data requires a named list with structured inputs, this proposes allowing the same for unconstrained parameters

there is no way of checking that the flattened structure is serialized correctly - i.e., column-major.

This is also one of the reasons why this proposal would be beneficial, as users currently have to perform this serialisation themselves. By handling this internally, and including validation checks, we can easily remove a source of user error.

a meta-objection is that this feature is unecessary

We've already had a request for it. Additionally, it's a very simple implementation:

  • Check parameter dimensions
  • Reorder parameters if needed
  • Call unlist()

Imo, a trivial amount of work to improve user experience

@mitzimorris
Copy link
Member

perhaps I misunderstood the proposal - what does the input for a matrix look like?

also, no feature is a trivial amount of work - in order to avoid future maintenance headaches, it must be thoroughly documented and tested.

@andrjohns
Copy link
Collaborator Author

what does the input for a matrix look like?

The same as for data and initial values (either an R matrix or dataframe), we then handle the serialisation internally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants