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

[REVIEW] RF: fix stream bug causing performance regressions #4644

Merged
merged 1 commit into from
Mar 23, 2022

Conversation

venkywonka
Copy link
Contributor

@venkywonka venkywonka commented Mar 22, 2022

This nanoPR fixes performance regression caused due to improper stream assignments to the decision trees.

Before fix:

sno algo input cu_time cpu_time cuml_acc cpu_acc speedup n_samples n_features max_depth n_estimators n_bins n_streams n_jobs n_classes
0 RandomForestClassifier numpy 32.635321855545044 0.0 0.99468 0.0 0.0 800000 64 8 500 128 4 -1 2
1 RandomForestClassifier numpy 40.36453413963318 0.0 0.994855 0.0 0.0 800000 64 10 500 128 4 -1 2
2 RandomForestClassifier numpy 61.35148477554321 0.0 0.99504 0.0 0.0 800000 64 16 500 128 4 -1 2

After fix:

sno algo input cu_time cpu_time cuml_acc cpu_acc speedup n_samples n_features max_depth n_estimators n_bins n_streams n_jobs n_classes
0 RandomForestClassifier numpy 28.637776374816895 0.0 0.99468 0.0 0.0 800000 64 8 500 128 4 -1 2
1 RandomForestClassifier numpy 34.11380743980408 0.0 0.994855 0.0 0.0 800000 64 10 500 128 4 -1 2
2 RandomForestClassifier numpy 47.153409481048584 0.0 0.99504 0.0 0.0 800000 64 16 500 128 4 -1 2

Command run in cuml/

python python/cuml/run_benchmarks.py--num-rows 800000 --num-features 64 --skip-cpu --test-split 0.2 --cuml-param-sweep "n_bins=[128]" "n_streams=[4]" --cpu-param-sweep "n_jobs=[-1]" --param-sweep "max_depth=[8,10,16]" "n_estimators=[500]" --n-reps 1 --csv pool-2112-cls-800000.csv --dataset-param-sweep "n_classes=[2]" --dtype "fp32" --dataset classification -- RandomForestClassifier

@venkywonka venkywonka requested a review from a team as a code owner March 22, 2022 13:59
@venkywonka venkywonka added ! - Hotfix Hotfix is a bug that affects the majority of users for which there is no reasonable workaround non-breaking Non-breaking change 3 - Ready for Review Ready for review by team Perf Related to runtime performance of the underlying code bug Something isn't working labels Mar 22, 2022
@venkywonka venkywonka changed the title [BUG] RF: fix stream bug causing performance regressions [REVIEW] RF: fix stream bug causing performance regressions Mar 22, 2022
@dantegd dantegd removed the ! - Hotfix Hotfix is a bug that affects the majority of users for which there is no reasonable workaround label Mar 22, 2022
@dantegd dantegd added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Mar 22, 2022
@dantegd
Copy link
Member

dantegd commented Mar 23, 2022

rerun tests

@dantegd
Copy link
Member

dantegd commented Mar 23, 2022

@gpucibot merge

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.04@48120ce). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-22.04    #4644   +/-   ##
===============================================
  Coverage                ?   83.86%           
===============================================
  Files                   ?      251           
  Lines                   ?    20293           
  Branches                ?        0           
===============================================
  Hits                    ?    17018           
  Misses                  ?     3275           
  Partials                ?        0           
Flag Coverage Δ
dask 44.92% <0.00%> (?)
non-dask 77.02% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out 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 48120ce...835c043. Read the comment docs.

@rapids-bot rapids-bot bot merged commit baf1637 into rapidsai:branch-22.04 Mar 23, 2022
@venkywonka venkywonka deleted the bug-ext-rf-streams branch March 23, 2022 06:13
vimarsh6739 pushed a commit to vimarsh6739/cuml that referenced this pull request Oct 9, 2023
This nanoPR fixes performance regression caused due to improper stream assignments to the decision trees.


Before fix:

 | sno |  algo | input | cu_time | cpu_time | cuml_acc | cpu_acc | speedup | n_samples | n_features | max_depth | n_estimators | n_bins | n_streams | n_jobs | n_classes | 
 | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | 
 | 0 | RandomForestClassifier | numpy | 32.635321855545044 | 0.0 | 0.99468 | 0.0 | 0.0 | 800000 | 64 | 8 | 500 | 128 | 4 | -1 | 2 | 
 | 1 | RandomForestClassifier | numpy | 40.36453413963318 | 0.0 | 0.994855 | 0.0 | 0.0 | 800000 | 64 | 10 | 500 | 128 | 4 | -1 | 2 | 
 | 2 | RandomForestClassifier | numpy | 61.35148477554321 | 0.0 | 0.99504 | 0.0 | 0.0 | 800000 | 64 | 16 | 500 | 128 | 4 | -1 | 2 | 

After fix:

| sno | algo | input | cu_time | cpu_time | cuml_acc | cpu_acc | speedup | n_samples | n_features | max_depth | n_estimators | n_bins | n_streams | n_jobs | n_classes
| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- |
0 | RandomForestClassifier | numpy | 28.637776374816895 | 0.0 | 0.99468 | 0.0 | 0.0 | 800000 | 64 | 8 | 500 | 128 | 4 | -1 | 2
1 | RandomForestClassifier | numpy | 34.11380743980408 | 0.0 | 0.994855 | 0.0 | 0.0 | 800000 | 64 | 10 | 500 | 128 | 4 | -1 | 2
2 | RandomForestClassifier | numpy | 47.153409481048584 | 0.0 | 0.99504 | 0.0 | 0.0 | 800000 | 64 | 16 | 500 | 128 | 4 | -1 | 2

Command run in `cuml/`
```
python python/cuml/run_benchmarks.py--num-rows 800000 --num-features 64 --skip-cpu --test-split 0.2 --cuml-param-sweep "n_bins=[128]" "n_streams=[4]" --cpu-param-sweep "n_jobs=[-1]" --param-sweep "max_depth=[8,10,16]" "n_estimators=[500]" --n-reps 1 --csv pool-2112-cls-800000.csv --dataset-param-sweep "n_classes=[2]" --dtype "fp32" --dataset classification -- RandomForestClassifier
```

Authors:
  - Venkat (https://github.com/venkywonka)

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: rapidsai#4644
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working CUDA/C++ non-breaking Non-breaking change Perf Related to runtime performance of the underlying code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants