Skip to content

Feature Request: Allow fit_xy for gen_additive_mod when not using the "mgcv" engine #775

New issue

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

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

Already on GitHub? # to your account

Closed
ben-e opened this issue Jul 22, 2022 · 2 comments · Fixed by #1103
Closed

Feature Request: Allow fit_xy for gen_additive_mod when not using the "mgcv" engine #775

ben-e opened this issue Jul 22, 2022 · 2 comments · Fixed by #1103
Assignees
Labels
feature a feature request or enhancement

Comments

@ben-e
Copy link

ben-e commented Jul 22, 2022

The current implementation of gen_additive_mod disables fit_xy due to the "mgcv" engine's use of formulas to specify smooths. I think this restriction is too mgcv specific, and may cause issues for other engines which specify smooths outside of the formula. Ideally, fit_xy could be disabled when engine = "mgcv", and enabled otherwise.

As an example, I am currently working on a package, wand, which allows users to specify smooths outside of a formula (example follows). I've currently got it running as its own model, nn_additive_mod, to work around the fit_xy restriction, but I think it would be a good candidate engine for gen_additive_mod.

# install.packages("pak")
# pak::pak("ben-e/wand")
library(wand)
library(modeldata)
library(parsnip)
library(recipes)
library(workflows)

data(bivariate)

# Using a workflow
wand_recipe <- recipe(Class ~ A + B, data = bivariate_train)
wand_recipe <- step_log(wand_recipe, all_numeric_predictors())

wand_spec <- nn_additive_mod(engine = "wand",
                             mode = "classification",
                             # Smooths are specified within a list, note that
                             # `B` here has already been log-transformed by
                             # the recipe.
                             smooth_specs = list(s_mlp(B)))

wand_wf <- workflow(preprocessor = wand_recipe, spec = wand_spec)

wand_fit <- fit(wand_wf, data = bivariate_train)

# Using a formula
wand_spec <- nn_additive_mod(engine = "wand",
                             mode = "regression",
                             # Rather than log-transforming with a recipe we
                             # can just do it in the smooth spec.
                             # If we want to specify the A ~ log(B) in the
                             # fit formula, then the smooth needs to be
                             # specified as s_mlp(`log(B)`).
                             smooth_specs = list(s_mlp(log(B))))

wand_fit <- fit(wand_spec,
                A ~ B,
                data = bivariate_train)

# This unfortunately does not translate well because nn_additive_mod is using
# the fit_xy interface which translates the formula to a dataframe prior to
# calling wand.
# As such, the fit formula can't accept something like A ~ s_mlp(log(B)),
# unlike gen_additive_mod which can accept a formula like `A ~ s(log(B))`
# In this case I'd just recommend users use wand directly:
# wand(A ~ s_mlp(log(B)))

I'd love to hear what ya'll think, thank you!

Ben

@ben-e ben-e changed the title Allow fit_xy for gen_additive_mod when not using the "mgcv" engine Feature Request: Allow fit_xy for gen_additive_mod when not using the "mgcv" engine Jul 22, 2022
@EmilHvitfeldt EmilHvitfeldt added the feature a feature request or enhancement label Mar 30, 2023
@topepo
Copy link
Member

topepo commented Apr 27, 2023

Sorry for the long wait.

We can remove that constraint on the overall class of models and just constrain the mgcv engine.

It will take me a few days to do this; currently in the weeds.

Copy link

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 29, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
feature a feature request or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants