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

[python-package] ignore training set on early stopping callback (fixes #5354) #5412

Merged
merged 5 commits into from
Aug 28, 2022

Conversation

jmoralez
Copy link
Collaborator

@jmoralez jmoralez commented Aug 10, 2022

Fixes #5354

@jmoralez jmoralez added the fix label Aug 11, 2022
@jmoralez jmoralez changed the title WIP: [python-package] ignore training set on early stopping callback [python-package] ignore training set on early stopping callback (fixes #5354) Aug 11, 2022
@jmoralez jmoralez marked this pull request as ready for review August 11, 2022 19:43

eval_records = {}
callbacks = [
lgb.record_evaluation(eval_records),
lgb.log_evaluation(2),
lgb.early_stopping(4)
lgb.early_stopping(10)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I changed this because otherwise it would stop at the 4th iteration, this was due to the training set being ignored for early stopping but now early stopping is disabled if you only provide the training set as valid, so I had to provide a dummy validation set.

@@ -1124,11 +1124,6 @@ def fit_and_check(eval_set_names, metric_names, assumed_iteration, first_metric_
iter_min = min([iter_min_l1, iter_min_l2])
iter_min_valid1 = min([iter_valid1_l1, iter_valid1_l2])

# training data as eval_set
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we shouldn't test this anymore but I'm open to modify it by adding an extra validation set so that we can check the training results

Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Tests and logic changes look good to me. Thanks for working on this!

Please see a few small suggestions.

Co-authored-by: James Lamb <jaylamb20@gmail.com>
@jameslamb jameslamb self-requested a review August 25, 2022 18:24
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

very nice, thanks for this!

@StrikerRUS StrikerRUS merged commit e063dad into master Aug 28, 2022
@StrikerRUS StrikerRUS deleted the ignore-train-es branch August 28, 2022 00:10
@jameslamb jameslamb mentioned this pull request Oct 7, 2022
40 tasks
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[python-package] Early Stopping does not work as expected
3 participants