cmu-delphi / epipredict

Tools for building predictive models in epidemiology.
https://cmu-delphi.github.io/epipredict/
Other
8 stars 9 forks source link

`arx_fcast_epi_workflow` is not particularly user friendly #215

Open dsweber2 opened 1 year ago

dsweber2 commented 1 year ago

There are apparently existing OKrs about this, but I wanted to put in an issue as well. I was attempting to modify just one step in an existing workflow (to allow for more NA filling in the frosting layer). Hypothetically there is a way to do this by intervening between arx_fcast_epi_workflow and fit, but the models are buried far too deep (ewf$pre$actions$recipe$recipe$steps) to be usable by someone only somewhat familiar with the package.

An idea would be to include a function like get_step(workflow,n) which could be used to get or set the nth step in the workflow.

dsweber2 commented 1 year ago

this is adjacent to #73, and I suspect is best thought of as a docs request in light of that

rachlobay commented 1 year ago

Good point, David… I was looking into whether we should add something like this as it is a long rabbit hole to go down to get to the steps in a workflow using $… This isn’t merged yet (rather it is on the branch 73-update-layer), but do you think update (for steps) in combination with update_epi_recipe() could do the trick?

Here’s a quick example of what I was thinking

library(epiprocess)
library(dplyr)
library(recipes)

jhu <- case_death_rate_subset %>%
  filter(time_value > "2021-08-01") %>%
  dplyr::arrange(geo_value, time_value)

r <- epi_recipe(jhu) %>%
  step_epi_lag(death_rate, lag = c(0, 7, 14)) %>%
  step_epi_ahead(death_rate, ahead = 7) %>%
  step_epi_lag(case_rate, lag = c(0, 7, 14)) %>%
  step_naomit(all_predictors()) %>%
  step_naomit(all_outcomes(), skip = TRUE)

wf <- epi_workflow() %>%
  add_epi_recipe(r)

r$steps[[1]] <- update(r$steps[[1]], lag = c(0, 1:3, 7))
r

wf <- update_epi_recipe(wf, r) 
wf

If yeah, this is ok with you, then I think it would be good to include this stuff somewhere in a doc example and possibly in a vignette as it would definitely be good for a user to know.

dsweber2 commented 1 year ago

That seems like a reasonable approach if you have an epi_recipe you're modifying. If I'm understanding this correctly, if I started with arx_fcast_epi_workflow, the equivalent change to the the first step would be

axe <- arx_fcast_epi_workflow(jhu, "death_rate", c("death_rate", "case_rate"))
axe$pre$actions$recipe$recipe$steps[[1]] <- update(axe$pre$actions$recipe$recipe$steps[[1]], lag = c(0,1:3,7))
axe$pre

which seems a bit unwieldy.

rachlobay commented 1 year ago

Agreed. Hmmm.... I think to make the above work in as clean a format we can manage with what we have (on that branch), we could modify my code slightly by adding the extract_preprocessor() function to get the recipe.

rachlobay commented 1 year ago

What I mean by this is below... So, like before, we still have the base of the update recipe & workflow but now we've just also extracted the recipe before that. What do you think @dsweber2?

jhu <- case_death_rate_subset %>%
  dplyr::filter(time_value >= as.Date("2021-12-01"))

axe <- arx_fcast_epi_workflow(jhu, "death_rate", 
                              c("death_rate", "case_rate"))

library(workflows)
r <- extract_preprocessor(axe)

r$steps[[1]] <- update(r$steps[[1]], lag = c(0, 1:3, 7))
r

axe <- update_epi_recipe(axe, r) 
axe
dsweber2 commented 1 year ago

Seems good to me! Thanks for the extension