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

Update base margin dask #6155

Merged
merged 8 commits into from Sep 26, 2020
Merged

Conversation

kylejn27
Copy link
Contributor

Add base_margin to sklearn predict and predict_proba
Add output_margin to sklearn predict and predict_proba
Add base_margin to sklearn fit
Update _get_worker_x_ordered to _get_worker_parts_ordered. This allows us to get ordered base_margin to create the DMatrix object used for dispatched_predict
Add dask test to confirm this is functioning correctly. Mirrors the same test in test_with_sklearn

@kylejn27 kylejn27 changed the title Add base margin dask Update base margin dask Sep 23, 2020
@kylejn27
Copy link
Contributor Author

Adds a few more things to #6132

@trivialfis trivialfis self-requested a review September 23, 2020 16:50
python-package/xgboost/dask.py Outdated Show resolved Hide resolved
python-package/xgboost/dask.py Outdated Show resolved Hide resolved
* Address format.
* Remove unused var.
@codecov-commenter
Copy link

codecov-commenter commented Sep 25, 2020

Codecov Report

Merging #6155 into master will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6155      +/-   ##
==========================================
+ Coverage   78.83%   78.91%   +0.08%     
==========================================
  Files          12       12              
  Lines        3090     3102      +12     
==========================================
+ Hits         2436     2448      +12     
  Misses        654      654              
Impacted Files Coverage Δ
python-package/xgboost/dask.py 81.07% <100.00%> (+0.68%) ⬆️
python-package/xgboost/data.py 56.80% <0.00%> (+0.19%) ⬆️

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 c686bc0...972cd17. Read the comment docs.

@trivialfis
Copy link
Member

@kylejn27 Could you please help taking a look into my last commit?

@kylejn27
Copy link
Contributor Author

@trivialfis looks great! Thanks for adding that in

On a side note, are you using a python autoformatter like black? I'd love to install one for this project

@trivialfis
Copy link
Member

Thanks for the review.

I believe XGBoost is using pylint. My local installation of pylint caught some errors but not the one on CI. Maybe there are some regressions on the latest pylint version (Mine is older).

For auto formatting, I use yapf myself, but not a requirement for XGBoost.

@trivialfis
Copy link
Member

You can run make lint on XGBoost's root dir.

@kylejn27
Copy link
Contributor Author

I use yapf myself

Cool, I'll take a look at that, should make following style guidelines a bit easier. Black wanted to reformat everything completely differently 😞

@trivialfis trivialfis merged commit e6a238c into dmlc:master Sep 26, 2020
@trivialfis
Copy link
Member

Thanks for the PR!

@kylejn27 kylejn27 deleted the add-base-margin-dask branch September 28, 2020 14:25
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.

None yet

4 participants