-
Notifications
You must be signed in to change notification settings - Fork 3
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
Simplify linear baseline model and bump all R dependencies #190
Conversation
@@ -9,7 +9,6 @@ Depends: | |||
ccao, | |||
conflicted, | |||
dplyr, | |||
embed, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The embed package has super heavy dependencies (keras, tensorflow), so getting rid of it lightens the dependencies of the main lockfile quite a bit.
embed::step_lencode_glm( | ||
all_of(cat_vars), -has_role("ID"), | ||
outcome = vars(meta_sale_price) | ||
) %>% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropping this step lets us remove the embed
dependency, but it also changes the performance of the linear model. Tagging @ccao-jardine here just as an FYI.
@jeancochrane @wrridgeway I'm merging this for expediency, but take a look Monday really quick. |
This PR simplifies the linear baseline model by removing the effect encoder step used in its preprocessing pipeline and replacing it with standard one-hot features. This is to reduce the dependency weight of the pipeline, as the
embed
recipes package has enormous sub-dependencies.This PR also bumps all the R/renv dependencies used by each of the pipeline profiles to their latest version available on CRAN.