Created post processing steps for adding the forecast and target dates #55
Conversation
…for both of those postprocessing steps
Merge branch 'frosting' of https://github.com/cmu-delphi/epipredict into 38-post_add_forecast_date-_add_target_date # Conflicts: # NAMESPACE
| #' Postprocessing step to add the forecast date | ||
| #' | ||
| #' @param frosting a `frosting` postprocessor | ||
| #' @param forecast_date The forecast date to add as a column to the `epi_df`. |
There was a problem hiding this comment.
Oops. Sorry, I missed this. To my mind the forecast_date is the date on which the forecast is made. So, by default, it should be max(time_value) from the training data. The target_date should be "the date the forecast is for". So that one should be max(time_value) + ahead by default.
It looks like they're both the same currently right?
There was a problem hiding this comment.
Almost… Current defaults: forecast_date = max_time_value + ahead (where max_time_value is for test data), target_date = time_value + ahead (based on simple.forecasts.Rmd). But I will change to what you specified. A couple qs for that...
For forecast_date, to get the max time value from the training data, is that using mold from components? Then, I check if the forecast_date < as_of_date of the test data and throw a warning there (that says "forecast_date is less than the most recent update date of the data.”, yes?
For target_date, by default, that is the max time value in the in the test data + ahead or no?
There was a problem hiding this comment.
The difficulty here is that leading/lagging increments the time_value. So this could all be a bit dangerous. In an ideal setting:
target_datewould be the max time value in the test data + ahead, as you said.forecast_datewould be the max time value in the test data. (So seemingly, the date of the most recent data available to you). Acting like we have data up to (and including) today, and we produce a forecast today, then this should work.
However, lots of weird crud can happen that would screw these up. If data isn't available for today but only for yesterday, then that would throw things off. If we accidentally lead the test data, then it'll produce "future" time_values.
…nto 38-post_add_forecast_date-_add_target_date Update this branch to latest frosting pulls
…gin frosting) Merge branch 'frosting' of https://github.com/cmu-delphi/epipredict into 38-post_add_forecast_date-_add_target_date # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
Merge branch 'frosting' of https://github.com/cmu-delphi/epipredict into 38-post_add_forecast_date-_add_target_date # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
2 participants
Addressed Issue #38 by making
layer_add_forecast_date()andlayer_add_target_date()as well as tests for each. I wasn't 100% sure about what was intended for the target date, so I defined it as in the simple-forecasts.Rmd vignette (wheretarget_date = time_value + ahead), but perhaps that wasn't quite what was intended... Let me know and I can update it.Update: Currently, I don't think the preprocessor is passed along (so that, for ex.,
aheaddoesn't need to be provided by the user to be accessed in these postprocessing functions)... Please hold off on reviewing for now until that is done and I have updated these two functions accordingly.