tidymodels / hardhat

Construct Modeling Packages
https://hardhat.tidymodels.org
Other
101 stars 16 forks source link

Error in out$extras$final(predictors_extras, outcomes_extras): argument "outcomes_extras" is missing, with no default #200

Closed mdancho84 closed 2 years ago

mdancho84 commented 2 years ago

Hey Davis,

I'm getting an error with modeltime, modeltime.ensemble, and modeltime.resample that is with the new hardhat 1.0.0.

Error in out$extras$final(predictors_extras, outcomes_extras): argument "outcomes_extras" is missing, with no default

It looks like outcome_extras is required but for some reason is not getting passed and created making the new hardhat not backwards compatible.

I'm working on creating a reproducible example, but I want to create this as a placeholder.

Currently the solution is to downgrade to hardhat 0.2.0 via:

remotes::install_version("hardhat", version = "0.2.0")
mdancho84 commented 2 years ago

I believe this is where the issue is generated.

image

mdancho84 commented 2 years ago

I've figured out a bandaid solution to upgrade models that were made using older hardhat <1.0.0 - I need to overwrite:

Older versions of hardhat

image

New hardhat

image

mdancho84 commented 2 years ago

Solution

Instead of extracting the mold, I am now creating a new mold from the preprocessor and preprocessor template. May not be the most elegant solution, but should fix the issues in modeltime.ensemble and modeltime.resample.

Do this

preprocessor <- workflows::extract_preprocessor(workflow)
mld          <- hardhat::mold(preprocessor, preprocessor$template)
forged       <- hardhat::forge(new_data, mld$blueprint)

Not this (will error on old models)

mld          <- object %>% workflows::extract_mold()
forged       <- hardhat::forge(new_data, mld$blueprint)
DavisVaughan commented 2 years ago

It turns out that this affects all workflows (I think?) that use a recipe. That was not my intention, but I didn't catch this because we don't do any testing on "old" saved hardhat objects against new versions of hardhat.

A Mac specific reprex is:

Run this with hardhat 0.2.0:

# devtools::install_version("hardhat", "0.2.0")
library(tidymodels)

spline_rec <-
  recipe(mpg ~ ., data = mtcars) %>%
  step_ns(disp, deg_free = 2) %>%
  step_ns(wt, deg_free = 2)

lin_mod <-
  linear_reg() %>%
  set_engine("lm")

wf <- workflow(spline_rec, lin_mod)

model_fit <- fit(wf, mtcars)

path <- "~/Desktop/model.rds"

saveRDS(model_fit, file = path)

Then install hardhat 1.0.0, and restart R. Then run:

# hardhat 1.0.0
# install.packages("hardhat")
library(tidymodels)

path <- "~/Desktop/model.rds"

model_fit <- readRDS(path)

predict(model_fit, mtcars)
#> Error in out$extras$final(predictors_extras, outcomes_extras) : 
#>   argument "outcomes_extras" is missing, with no default

This occurs because hardhat stores function objects in the results of hardhat::mold(), which are then called at predict() time through hardhat::forge().

So you can get into a state where you are using an old model that was fit with one version of hardhat, but it is incompatible with a new version of hardhat just because I changed some of the internal details.

In #201 I've rewritten a large chunk of the internals to no longer store the function objects in the blueprint. They are looked up on the fly instead - which should ensure that we always get a "current" version of those functions.

I'll need to do one more follow up PR after that, but I'm fairly confident it'll fix this issue.

DavisVaughan commented 2 years ago

I think you should roll back this commit after I add the fix https://github.com/business-science/modeltime/commit/3453a6bc798e8d65fe925d465defb5b76f82b371#diff-fe5d8d66bcb3a2b014a57ce94e356a349340cf25f6f5cd1123864574f0205effR781

I don't think that re-calling mold() is a good idea, if you can help it

mdancho84 commented 2 years ago

Ah ok. I'll have to redo some stuff but now I have an idea of what's going on so it should be ok.

mdancho84 commented 2 years ago

Hey Davis, I have updated all of the modeltime R packages to pass CRAN and allow backwards compat (albeit not desirably with re-molding).

Once you release a new hardhat, I'll update modeltime to revert the remolding. So this way you have plenty of time to fix things without any issues on my end from CRAN. -Matt

DavisVaughan commented 2 years ago

@mdancho84, I've finished the work that should make the next version of hardhat compatible with workflows that were saved using a version of hardhat that was pre-1.0.0. Is there any way you can test this for me? I've done some tests on my end but nothing with modeltime.

I think you'd need to get the dev version of hardhat + the previous version of modeltime before your patches, and then run your original example with those installed

mdancho84 commented 2 years ago

I can test after the kids go down tonight. I have some old models that I will try with modeltime.

DavisVaughan commented 2 years ago

Thanks! I'll plan to send hardhat in tomorrow

mdancho84 commented 2 years ago

Hey Davis,

I ran the following tests:

I'd say you are good to go.

-Matt

mdancho84 commented 2 years ago

I'll probably wait the 30-days but eventually revert the "re-molding" versions of modeltime R packages.

DavisVaughan commented 2 years ago

Thanks, just sent it in. FYI the 30 day limit has been reduced to 6-7 days now for sending in a new release

mdancho84 commented 2 years ago

OK, I just got a warning from CRAN for updating in 6-days. But maybe I was just on the edge. Thanks for the heads up!

github-actions[bot] commented 2 years ago

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.