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

FIL and Dask demo #3698

Merged
merged 4 commits into from
Apr 2, 2021
Merged

FIL and Dask demo #3698

merged 4 commits into from
Apr 2, 2021

Conversation

miroenev
Copy link
Contributor

@miroenev miroenev commented Apr 1, 2021

  • Replaced numpy with cupy, and sklearn with cuml (e.g., data generation and splitting)
  • Resized XGBoost model params from 15 trees at 25 max_depth, to 100 trees at 20 max_depth
  • Added Dask distributed data generation, FIL model loading on worker init, and distributed FIL prediction

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added the Cython / Python Cython or Python issue label Apr 1, 2021
@miroenev miroenev requested review from wphicks and dantegd April 1, 2021 17:25
Copy link
Contributor

@wphicks wphicks left a comment

Choose a reason for hiding this comment

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

Really nice demo! I'm going to be referring to this one a lot in the future.

A few small items in comments. The only overall thing is don't forget to clear outputs before you finalize the PR to allow this to be run in CI.

notebooks/forest_inference_demo.ipynb Outdated Show resolved Hide resolved
notebooks/forest_inference_demo.ipynb Outdated Show resolved Hide resolved
notebooks/forest_inference_demo.ipynb Outdated Show resolved Hide resolved
@miroenev miroenev added 3 - Ready for Review Ready for review by team Multi-GPU Issues & PRs related to multi-GPU functionality Notebook Issue or PR related to cuML notebook examples and removed Cython / Python Cython or Python issue labels Apr 1, 2021
@miroenev miroenev requested a review from wphicks April 1, 2021 19:07
Copy link
Contributor

@wphicks wphicks left a comment

Choose a reason for hiding this comment

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

One last formatting thing in comments, but this LGTM!

notebooks/forest_inference_demo.ipynb Outdated Show resolved Hide resolved
@github-actions github-actions bot added the Cython / Python Cython or Python issue label Apr 1, 2021
Copy link
Contributor

@wphicks wphicks left a comment

Choose a reason for hiding this comment

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

Oops! Think the cell outputs came back

@miroenev
Copy link
Contributor Author

miroenev commented Apr 1, 2021

That was odd, should be cleared now. thanks!

Copy link
Contributor

@wphicks wphicks left a comment

Choose a reason for hiding this comment

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

LGTM!

@JohnZed JohnZed added non-breaking Non-breaking change doc Documentation labels Apr 1, 2021
@codecov-io
Copy link

Codecov Report

Merging #3698 (edeeecc) into branch-0.19 (fd9ec89) will increase coverage by 2.23%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.19    #3698      +/-   ##
===============================================
+ Coverage        80.70%   82.94%   +2.23%     
===============================================
  Files              227      227              
  Lines            17615    17726     +111     
===============================================
+ Hits             14217    14702     +485     
+ Misses            3398     3024     -374     
Flag Coverage Δ
dask 46.40% <ø> (+1.41%) ⬆️
non-dask 75.02% <ø> (+2.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
python/cuml/__init__.py 95.45% <ø> (+0.06%) ⬆️
...cuml/_thirdparty/sklearn/preprocessing/__init__.py 100.00% <ø> (ø)
...on/cuml/_thirdparty/sklearn/preprocessing/_data.py 64.40% <ø> (+1.29%) ⬆️
...hirdparty/sklearn/preprocessing/_discretization.py 83.33% <ø> (-0.88%) ⬇️
...l/_thirdparty/sklearn/preprocessing/_imputation.py 85.54% <ø> (+22.74%) ⬆️
python/cuml/_thirdparty/sklearn/utils/extmath.py 56.89% <ø> (ø)
...cuml/_thirdparty/sklearn/utils/skl_dependencies.py 80.00% <ø> (+26.07%) ⬆️
...ython/cuml/_thirdparty/sklearn/utils/validation.py 18.41% <ø> (-4.04%) ⬇️
python/cuml/cluster/__init__.py 100.00% <ø> (ø)
python/cuml/cluster/agglomerative.pyx 96.47% <ø> (ø)
... and 175 more

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 fb088d9...edeeecc. Read the comment docs.

@JohnZed
Copy link
Contributor

JohnZed commented Apr 2, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 2d59722 into rapidsai:branch-0.19 Apr 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team Cython / Python Cython or Python issue doc Documentation Multi-GPU Issues & PRs related to multi-GPU functionality non-breaking Non-breaking change Notebook Issue or PR related to cuML notebook examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants