[REVIEW] Fix for crash in RF when max_leaves parameter is specified

[vinaydes]

Fixes issue #4046.
In the nodeSplitKernel each thread calls leafBasedOnParams() which reads global variable n_leaves. Different threads from same threadblock read n_leaves at different times. Between two threads reading n_leaves, value of it could be changed by some other threadblock. Thus one or few threads might concluded that max_leaves is reached, and rest of the threads might conclude otherwise. This caused crash in partitioning the samples.

In the solution provided here, instead of every thread reading n_leaves, only one thread from a threadblock reads the value and broadcasts it to every other thread via shared memory. This ensures complete agreement on max_leaves criterion among threads from threadblock.

Performance results to be posted shortly.

[vinaydes]

Minimal performance regression observed overall, no loss of accuracy.
Detailed performance results:
For max_depth = 32

Average performance gain =  -0.19167074530609718 %
Classification accuracy =
  dataset_name  Baseline   PR 4126
0      airline  0.830460  0.830460
1        higgs  0.747569  0.747569
2      epsilon  0.755010  0.755010
Regression MSE =
         dataset_name    Baseline     PR 4126
0  airline_regression  419.453003  419.453003
1                year   88.178551   88.178551

For max_depth = 24

Average performance gain =  -0.12289140368544338 %
Classification accuracy =
  dataset_name  Baseline   PR 4126
0      airline  0.810743  0.810743
1        higgs  0.742905  0.742905
2      epsilon  0.756150  0.756150
Regression MSE =
         dataset_name    Baseline     PR 4126
0  airline_regression  418.358765  418.358765
1                year   87.964699   87.964699

For max_depth = 18

Average performance gain =  -0.2654329881113543
Classification accuracy =
  dataset_name  Baseline   PR 4126
0      airline  0.753953  0.753953
1        higgs  0.734163  0.734163
2      epsilon  0.760830  0.760830
Regression MSE =
         dataset_name    Baseline     PR 4126
0  airline_regression  433.178467  433.178467
1                year   87.441147   87.441147

The GBM-bench parameter file used for benchmarking can be located here.

[venkywonka]

awesome fix vinay! 🙏🏻

[teju85]

Changes LGTM. Thank you Vinay and Venkat for the fix.

[dantegd]

@gpucibot merge

[codecov-commenter]

Codecov Report

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

@@               Coverage Diff               @@
##             branch-21.08    #4126   +/-   ##
===============================================
  Coverage                ?   85.91%           
===============================================
  Files                   ?      232           
  Lines                   ?    18377           
  Branches                ?        0           
===============================================
  Hits                    ?    15788           
  Misses                  ?     2589           
  Partials                ?        0           

Flag	Coverage Δ
dask	48.03% <0.00%> (?)
non-dask	78.45% <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 9406d53...3a814fa. Read the comment docs.