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

[timeseries] fix random seed, reset seed for every model #3934

Merged
merged 6 commits into from
Feb 20, 2024

Conversation

canerturkmen
Copy link
Contributor

Issue #, if available:

#3896
#3923

Description of changes:

Previously, seed_everything was called once at the beginning of TimeSeriesPredictor fit and predict methods which led to two undesirable side effects:

  • if any of the models reset the seeds in their internal logic, they could effectively cancel the random_seed provided by the user. This was done, e.g., in RecursiveTabular's NN_TORCH affecting high_quality presets.
  • The fitting order of models could affect the outcomes of individual models. i.e., the random state at the beginning of each model depended on previous models.

With this change, we propagate the random seed to the learner and trainer both at training and prediction time. We set the seed again, per model, when fitting single models or predicting with single models.

Also fixes a minor issue in predictor unit tests.

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

@canerturkmen canerturkmen added bug Something isn't working module: timeseries related to the timeseries module labels Feb 19, 2024
Copy link
Collaborator

@shchur shchur left a comment

Choose a reason for hiding this comment

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

LGTM! Feel free to merge once the tests pass.

@shchur shchur added this to the 1.0.1 Release milestone Feb 20, 2024
@canerturkmen canerturkmen merged commit ab73555 into autogluon:master Feb 20, 2024
20 of 21 checks passed
@canerturkmen canerturkmen deleted the ts-fix-model-seeding branch February 20, 2024 12:27
@Innixma Innixma modified the milestones: 1.0.1 Release, 1.1 Release Apr 5, 2024
canerturkmen added a commit that referenced this pull request Apr 14, 2024
shchur pushed a commit that referenced this pull request Apr 15, 2024
LennartPurucker pushed a commit to LennartPurucker/autogluon that referenced this pull request Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working module: timeseries related to the timeseries module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants