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 r_forecast wrapper to shift start date when truncating time series #2216

Merged
merged 4 commits into from
Aug 17, 2022

Conversation

abdulfatir
Copy link
Contributor

Issue #, if available:

Description of changes:

Fixed the r_forecast wrapper to shift the start date of the time series accordingly when truncating the time series. Currently, it uses the the original start date even for the truncated time series which results in an incorrect forecast start date.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Please tag this pr with at least one of these labels to make our release process faster: BREAKING, new feature, bug fix, other change, dev setup

@lostella lostella added pending v0.9.x backport pending v0.10.x backport This contains a fix to be backported to the v0.10.x branch bug fix (one of pr required labels) labels Aug 15, 2022
@lostella
Copy link
Contributor

Thanks @abdulfatir! I think we should probably also update this test to cover also non-default values of trunc_length. I guess that test could have caught this, right?

@abdulfatir
Copy link
Contributor Author

@lostella Updated test.

Copy link
Contributor

@lostella lostella left a comment

Choose a reason for hiding this comment

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

Thanks @abdulfatir! Looks good to me, it seems like tests for these models are not running in the GitHub workflows, but we will fix this separately.

@lostella lostella changed the title Fix r_forecast wrapper to shift start date when truncating time series Fix r_forecast wrapper to shift start date when truncating time series Aug 17, 2022
@lostella lostella merged commit b47a602 into awslabs:dev Aug 17, 2022
@abdulfatir abdulfatir deleted the r-forecast-bug branch August 17, 2022 19:24
lostella pushed a commit to lostella/gluonts that referenced this pull request Aug 26, 2022
@lostella lostella mentioned this pull request Aug 26, 2022
lostella pushed a commit to lostella/gluonts that referenced this pull request Aug 26, 2022
@lostella lostella mentioned this pull request Aug 26, 2022
@lostella lostella removed pending v0.9.x backport pending v0.10.x backport This contains a fix to be backported to the v0.10.x branch labels Aug 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix (one of pr required labels)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants