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

Formula parsing error with t2() interaction terms #102

Closed
hsbadr opened this issue Apr 3, 2021 · 2 comments
Closed

Formula parsing error with t2() interaction terms #102

hsbadr opened this issue Apr 3, 2021 · 2 comments

Comments

@hsbadr
Copy link
Member

hsbadr commented Apr 3, 2021

varsel()/make_formula fails with t2() interaction terms (see paul-buerkner/brms#1133).

Here's a simple reproducible example:

library(brms)
library(projpred)

dat <- mgcv::gamSim(1, n = 400, scale = 2)
fit1 <- brm(y ~ x1 + t2(x0, x2), data = dat)

refmod1 <- get_refmodel(fit1)
vs1 <- varsel(refmod1)

Error in str2lang(x) : :1:29: unexpected ','
1: . ~ 1 + t2(x0, x2) + x1 + x0,
^

Here's the traceback:

> traceback()
9: str2lang(x)
8: formula.character(object, env = baseenv())
7: formula(object, env = baseenv())
6: as.formula(paste0(". ~ ", paste(terms_, collapse = " + "))) at formula.R#595
5: make_formula(list_of_terms) at formula.R#729
4: count_terms_chosen(search_terms, duplicates = TRUE) at varsel.R#327
3: parse_args_varsel(refmodel, method, cv_search, intercept, nterms_max,
       nclusters, ndraws, nclusters_pred, ndraws_pred, search_terms) at varsel.R#118
2: varsel.refmodel(refmod1) at varsel.R#93
1: varsel(refmod1)

A quick debug reveals that projpred:::make_formula() uses a term ("x0, x2") generated by projpred:::split_formula() that includes the two variables (x0 and x2) separated by a comma, which causes the failure:

> projpred:::split_formula(refmod1$formula)
[1] "1"          "t2(x0, x2)" "x1"         "x0, x2"

> paste0(". ~ ", paste(projpred:::split_formula(refmod1$formula), collapse = " + "))
[1] ". ~ 1 + t2(x0, x2) + x1 + x0, x2"

Either the added term ("x0, x2") needs to be removed or the comma replaced by +.

@AlejandroCatalina
Copy link
Collaborator

Exactly, we did not implement more complex splines. Thanks for the PR, I'll look into it now!

@AlejandroCatalina
Copy link
Collaborator

Fixed in latest develop.

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

No branches or pull requests

2 participants