Skip to content

380 add forecast date recipe #433

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

Merged
merged 13 commits into from
Feb 4, 2025
Merged

380 add forecast date recipe #433

merged 13 commits into from
Feb 4, 2025

Conversation

dajmcdon
Copy link
Contributor

Checklist

Please:

  • Make sure this PR is against "dev", not "main".
  • Request a review from one of the current epipredict main reviewers:
    dajmcdon.
  • Make sure to bump the version number in DESCRIPTION and NEWS.md.
    Always increment the patch version number (the third number), unless you are
    making a release PR from dev to main, in which case increment the minor
    version number (the second number).
  • Describe changes made in NEWS.md, making sure breaking changes
    (backwards-incompatible changes to the documented interface) are noted.
    Collect the changes under the next release number (e.g. if you are on
    0.7.2, then write your changes under the 0.8 heading).
  • Consider pinning the epiprocess version in the DESCRIPTION file if
    • You anticipate breaking changes in epiprocess soon
    • You want to co-develop features in epipredict and epiprocess

Change explanations for reviewer

Added reference_date to the epi recipe. I think this is likely a better terminology. The hope is to use these for nowcasts and backcasts, so the thinking here is to make downstream computations relative to the reference in all cases. It's not specific only to forecasts.

Note: Downstream steps (such as step_adjust_latency()) can be updated in separate PRs.

Magic GitHub syntax to mark associated Issue(s) as resolved when this is merged into the default branch

@dajmcdon dajmcdon linked an issue Jan 31, 2025 that may be closed by this pull request
@dajmcdon dajmcdon requested a review from dsweber2 January 31, 2025 18:47
Copy link
Contributor

@dsweber2 dsweber2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible problem down the road: Roni is talking about switching time_value for reference_date in epidata, so we may want to too, which would lead to some confusing name collision. Probably worth going ahead with this anyways.

Just confirming, no steps/layers are currently using this right? E.g. layer_add_forecast_date?

@dajmcdon
Copy link
Contributor Author

dajmcdon commented Feb 1, 2025

  1. Correct. This is not currently being used by any layers or steps downstream. We can make those changes later.
  2. On the reference_date name, I considered forecast_date (as in the issue name), but that seems poor relative to other non-forecasting tasks. I also thought about training_date, but the model technically isn't trained until it is fit(), so that doesn't seem right either. Maybe model_creation_date? I like reference_date because it's an important piece of information to which other methods may refer. But I'm open to alternatives.

@dsweber2
Copy link
Contributor

dsweber2 commented Feb 3, 2025

reference_date would definitely be the best option without the collision. model_date might work; I think model_creation_date may actually be pointing to the day when the model is fit rather than the forecast date (mostly comes up in backtesting). Otherwise forecast_date seems clear enough even if it might not be an exact fit for backfill projection and nowcasting. Possibly prediction_date?

@dajmcdon dajmcdon merged commit 18d1725 into dev Feb 4, 2025
2 checks passed
@dajmcdon dajmcdon deleted the 380-add-forecast_date-recipe branch February 4, 2025 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add forecast_date as a first-class part of the recipe object
2 participants