Skip to content
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

Fix weekly date range bug for deepar #154

Merged

Conversation

es94129
Copy link
Contributor

@es94129 es94129 commented Nov 14, 2024

What is changed?

Specify the "W-{weekday}" instead of "W" for the pd.date_range frequency, so that there is no mismatch between the new index and the original time column's dates.

  • Previously: new_index_full can start from 2010-02-07 (a Sunday) despite total_min is 2010-02-05 (a Friday), because pd.date_range assumes that a weekly frequency starts from a Sunday (W-SUN)
  • Now: the start of new_index_full is aligned with total_min

How is it tested?

  • Added unit test: provide a dataframe where the dates are Fridays, and the frequency is "W". Before the fix, the unit test would fail with the target column of the output dataframe(s) being all nan.
  • Manual testing: tested with the Walmart Sales dataset, the DeepAR metrics used to be NaN before the change
image

@es94129 es94129 requested a review from sabhya-db November 15, 2024 00:15
Copy link

@sabhya-db sabhya-db left a comment

Choose a reason for hiding this comment

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

lgtm

@es94129 es94129 merged commit de71cd8 into databricks:branch-0.2.20.5 Nov 15, 2024
2 checks passed
@es94129 es94129 deleted the es94129/fix-deepar-weekday branch November 15, 2024 21:23
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.

2 participants