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.

Sign up for GitHub

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

Jump to bottom

[timeseries] Fix cached predictions error when predictor saved to directory with an existing predictor #4202

Merged
merged 2 commits into from
May 16, 2024
Merged

[timeseries] Fix cached predictions error when predictor saved to directory with an existing predictor #4202

merged 2 commits into from
May 16, 2024

Conversation

shchur
Copy link
Collaborator

@shchur shchur commented May 15, 2024

Issue #, if available: Fixes #4183 #4195

Description of changes:

  • Currently there is a bug in cached prediction logic that occurs if we re-use a directory where a predictor has been trained before. Specifically, the trainer tries to load some metadata for each model mentioned in the cached_predictions.pkl dictionary. However, if the model was trained by the old predictor (but isn't trained by the new predictor), this will result in an error.
  • We fix this problem by making sure that the cached predictions are deleted whenever a new trainer is created.

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

@shchur shchur added this to the 1.1.1 Release milestone May 15, 2024
@shchur shchur added bug Something isn't working module: timeseries related to the timeseries module labels May 15, 2024
@yinweisu
Copy link
Collaborator

Previous CI Run Current CI Run
botocore==1.34.105 botocore==1.34.106
regex==2024.5.10 regex==2024.5.15
boto3==1.34.105 boto3==1.34.106
platformdirs==4.2.1 platformdirs==4.2.2
botocore==1.34.105 botocore==1.34.106
regex==2024.5.10 regex==2024.5.15
boto3==1.34.105 boto3==1.34.106
platformdirs==4.2.1 platformdirs==4.2.2

@github-actions GitHub Actions
Copy link

Job PR-4202-fc47ac7 is done.
Docs are uploaded to http://autogluon-staging.s3-website-us-west-2.amazonaws.com/PR-4202/fc47ac7/index.html

Innixma
Innixma approved these changes May 15, 2024
View reviewed changes
Copy link
Contributor

@Innixma Innixma left a comment

Choose a reason for hiding this comment

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

LGTM!

timeseries/tests/unittests/test_predictor.py Outdated Show resolved Hide resolved
@yinweisu
Copy link
Collaborator

Previous CI Run Current CI Run

@github-actions GitHub Actions
Copy link

Job PR-4202-350ec59 is done.
Docs are uploaded to http://autogluon-staging.s3-website-us-west-2.amazonaws.com/PR-4202/350ec59/index.html

@shchur shchur merged commit 2d28429 into autogluon:master May 16, 2024
29 checks passed
@shchur shchur deleted the cached-predictions-ts-fix branch May 16, 2024 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@canerturkmen canerturkmen canerturkmen approved these changes

@Innixma Innixma Innixma approved these changes

Assignees
No one assigned
Labels
bug Something isn't working module: timeseries related to the timeseries module
Projects
None yet
Milestone
1.1.1 Release
Development

Successfully merging this pull request may close these issues.

networkx.exception.NetworkXError: The node ETS is not in the digraph.
4 participants
@shchur @yinweisu @canerturkmen @Innixma