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

Add a repair_variable_names function #317

Open
WardBrian opened this issue Nov 17, 2023 · 8 comments · May be fixed by #318
Open

Add a repair_variable_names function #317

WardBrian opened this issue Nov 17, 2023 · 8 comments · May be fixed by #318
Labels
feature New feature or request

Comments

@WardBrian
Copy link
Member

This allows you to translate from the names Stan uses (like theta.1) to those Posterior wants (theta[1]).

This function exists (privately) in cmdstanr, and is currently very simple:

repair_variable_names <- function(names) {
    names <- sub("\\.", "[", names)
    names <- gsub("\\.", ",", names)
    names[grep("\\[", names)] <- paste0(names[grep("\\[", names)], "]")
    names
}

This doesn't work for complex numbers or tuples, but since posterior doesn't seem to support those either I think this is fine

@jgabry
Copy link
Member

jgabry commented Nov 17, 2023

If the other posterior authors are ok with adding this I can make a PR.

@jgabry jgabry added the feature New feature or request label Nov 17, 2023
@paul-buerkner
Copy link
Collaborator

paul-buerkner commented Nov 17, 2023 via email

@jgabry
Copy link
Member

jgabry commented Nov 17, 2023

@paul-buerkner Any preference where this should go? I was thinking of documenting it on the same doc page as set_variables(), but I could also put it somewhere else. On slack @avehtari gave an example of

x <- set_variables(x, repair_variable_names(bad_names))

which seems like the most common use case for a function like repair_variable_names.

@paul-buerkner
Copy link
Collaborator

paul-buerkner commented Nov 17, 2023 via email

jgabry added a commit that referenced this issue Nov 17, 2023
@jgabry jgabry linked a pull request Nov 17, 2023 that will close this issue
@avehtari
Copy link
Collaborator

Started to think, whether repair is the best name? Would convert be better?

@avehtari
Copy link
Collaborator

This doesn't work for complex numbers or tuples, but since posterior doesn't seem to support those either I think this is fine

Can you add examples of how the names for these would look like? It would be good to recognize complex and tuples even if there would not be support at the moment

@jgabry
Copy link
Member

jgabry commented Nov 17, 2023

Started to think, whether repair is the best name? Would convert be better?

I opened a PR for this (#318) using repair but I'm happy to change the name. Do you want to move the naming discussion to the PR review?

@WardBrian
Copy link
Member Author

Can you add examples of how the names for these would look like? It would be good to recognize complex and tuples even if there would not be support at the moment

Complex variables use .real and .imag as their CSV names in Stan. So something like a complex_vector[2] foo has names foo.1.real,foo.1.imag,foo.2.real,foo.2.imag

Tuples use : to separate "slots" in a tuple. So a tuple(real, int) a is printed as a:1,a:2. This stacks naturally with the . for arrays of tuples/tuples of arrays.

If you want some really nasty edge cases, you can see some of the test cases of my stanio Python package

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

Successfully merging a pull request may close this issue.

4 participants