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

Rework Python callback functions. #6199

Merged
merged 53 commits into from Oct 10, 2020
Merged

Conversation

trivialfis
Copy link
Member

This PR adds a new callback function interface for Python, also a new type of callback for checkpointing is added.

Please see documentation and demo for details. I added a demo for plotting animated evaluation scores, try it out. ;-)

Related #5612 I haven't made the parameter to dask. The PR is already huge as it is.
Related #3822 this is only for Python. Other language bindings are not supported.
Close #4955
Close #5495 now we can use early stopping on dask.

@trivialfis trivialfis mentioned this pull request Oct 4, 2020
14 tasks
@hcho3
Copy link
Collaborator

hcho3 commented Oct 4, 2020

@trivialfis Is this ready for review? Do you want me to review this?

@trivialfis
Copy link
Member Author

@hcho3 I ran some of the tests locally. Feel free to review once the CI is green.

@hcho3
Copy link
Collaborator

hcho3 commented Oct 4, 2020

CI might fail due to a Conda issue. Related: https://xgboost-ci.net/blue/organizations/jenkins/xgboost/detail/PR-6198/1/pipeline

doc/python/callbacks.rst Outdated Show resolved Hide resolved
python-package/xgboost/training.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Oct 6, 2020

Codecov Report

Merging #6199 into master will increase coverage by 0.82%.
The diff coverage is 95.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6199      +/-   ##
==========================================
+ Coverage   78.93%   79.75%   +0.82%     
==========================================
  Files          12       12              
  Lines        3104     3359     +255     
==========================================
+ Hits         2450     2679     +229     
- Misses        654      680      +26     
Impacted Files Coverage Δ
python-package/xgboost/training.py 96.41% <94.59%> (+0.89%) ⬆️
python-package/xgboost/callback.py 88.39% <96.10%> (+0.39%) ⬆️
python-package/xgboost/dask.py 81.10% <100.00%> (+0.03%) ⬆️
python-package/xgboost/sklearn.py 91.68% <0.00%> (+0.24%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1013224...cc1ec18. Read the comment docs.

@trivialfis trivialfis requested a review from hcho3 October 7, 2020 06:48
@trivialfis
Copy link
Member Author

Note to myself:

Maybe add a hook for customizable allreduce.

Copy link
Member

@RAMitchell RAMitchell left a comment

Choose a reason for hiding this comment

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

I like this reorganisation. Nice to see checkpointing as a callback, and very nice demo with the plot!

Copy link
Contributor

@JohnZed JohnZed left a comment

Choose a reason for hiding this comment

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

Looks good! Just added very trivial typo suggestion

doc/python/callbacks.rst Outdated Show resolved Hide resolved
doc/python/callbacks.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@hcho3 hcho3 left a comment

Choose a reason for hiding this comment

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

LGTM. This pull request is a great testament to the power of good abstractions in programming. Some minor comments follow:

demo/guide-python/callbacks.py Outdated Show resolved Hide resolved
python-package/xgboost/training.py Show resolved Hide resolved
python-package/xgboost/training.py Outdated Show resolved Hide resolved
python-package/xgboost/training.py Show resolved Hide resolved
@trivialfis
Copy link
Member Author

@terrytangyuan @RAMitchell @JohnZed @hcho3 Thanks for the review!

@codecov-io
Copy link

codecov-io commented Oct 10, 2020

Codecov Report

Merging #6199 into master will increase coverage by 1.31%.
The diff coverage is 95.98%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6199      +/-   ##
==========================================
+ Coverage   78.93%   80.24%   +1.31%     
==========================================
  Files          12       12              
  Lines        3104     3362     +258     
==========================================
+ Hits         2450     2698     +248     
- Misses        654      664      +10     
Impacted Files Coverage Δ
python-package/xgboost/training.py 96.46% <94.80%> (+0.94%) ⬆️
python-package/xgboost/callback.py 92.61% <96.10%> (+4.61%) ⬆️
python-package/xgboost/dask.py 81.10% <100.00%> (+0.03%) ⬆️
python-package/xgboost/sklearn.py 91.68% <0.00%> (+0.24%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5b2435...8e45139. Read the comment docs.

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.

[FEA] DaskXGBClassifier Early Stopping Learning rate decay
7 participants