Repository navigation
Implement vector leaf for random forest #4191
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
Changes from all commits
812d9c7
2ac77c3
735c983
b077cf3
e54fb6d
1f88053
af0aede
8e57f41
3c9a27b
14cb0f5
26362b0
bc0934f
f288814
c7102ea
9a0edb9
6328fe8
ce3221c
5019199
919244b
a592b12
7f9c219
6f76bee
93e6d1d
a274aa2
bb61c47
471bda4
7ed733f
e9525c3
906dff8
8ebef30
1f59a24
138ca0e
289626a
ab6017d
779fd93
e9863ec
73dba17
f192c03
ee1cb59
5167bd9
96abb0c
dfa43dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,7 +73,7 @@ class GiniObjectiveFunction { | |
| constexpr DataT One = DataT(1.0); | ||
| DataT invlen = One / len; | ||
| for (IdxT i = threadIdx.x; i < nbins; i += blockDim.x) { | ||
| int nLeft = 0; | ||
| IdxT nLeft = 0; | ||
| for (IdxT j = 0; j < nclasses; ++j) { | ||
| nLeft += scdf_labels[nbins * j + i].x; | ||
| } | ||
|
|
@@ -106,18 +106,16 @@ class GiniObjectiveFunction { | |
| } | ||
| return sp; | ||
| } | ||
| static DI LabelT LeafPrediction(BinT* shist, int nclasses) | ||
| static DI void SetLeafVector(BinT* shist, int nclasses, DataT* out) | ||
| { | ||
| int class_idx = 0; | ||
| int count = 0; | ||
| // Output probability | ||
| int total = 0; | ||
| for (int i = 0; i < nclasses; i++) { | ||
| auto current_count = shist[i].x; | ||
| if (current_count > count) { | ||
| class_idx = i; | ||
| count = current_count; | ||
| } | ||
| total += shist[i].x; | ||
| } | ||
| for (int i = 0; i < nclasses; i++) { | ||
| out[i] = DataT(shist[i].x) / total; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This division can be avoided by first computing the inverse of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This calculation is performed only once per leaf, per class. Are you sure this optimisation is necessary? |
||
| } | ||
| return class_idx; | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -146,7 +144,7 @@ class EntropyObjectiveFunction { | |
| constexpr DataT One = DataT(1.0); | ||
| DataT invlen = One / len; | ||
| for (IdxT i = threadIdx.x; i < nbins; i += blockDim.x) { | ||
| int nLeft = 0; | ||
| IdxT nLeft = 0; | ||
| for (IdxT j = 0; j < nclasses; ++j) { | ||
| nLeft += scdf_labels[nbins * j + i].x; | ||
| } | ||
|
|
@@ -185,10 +183,16 @@ class EntropyObjectiveFunction { | |
| } | ||
| return sp; | ||
| } | ||
| static DI LabelT LeafPrediction(BinT* shist, int nclasses) | ||
| static DI void SetLeafVector(BinT* shist, int nclasses, DataT* out) | ||
| { | ||
| // Same as Gini | ||
| return GiniObjectiveFunction<DataT, LabelT, IdxT>::LeafPrediction(shist, nclasses); | ||
| // Output probability | ||
| int total = 0; | ||
| for (int i = 0; i < nclasses; i++) { | ||
| total += shist[i].x; | ||
| } | ||
| for (int i = 0; i < nclasses; i++) { | ||
| out[i] = DataT(shist[i].x) / total; | ||
| } | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -260,9 +264,11 @@ class MSEObjectiveFunction { | |
| return sp; | ||
| } | ||
|
|
||
| static DI LabelT LeafPrediction(BinT* shist, int nclasses) | ||
| static DI void SetLeafVector(BinT* shist, int nclasses, DataT* out) | ||
| { | ||
| return shist[0].label_sum / shist[0].count; | ||
| for (int i = 0; i < nclasses; i++) { | ||
| out[i] = shist[i].label_sum / shist[i].count; | ||
| } | ||
| } | ||
| }; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just confirming, are we allocating twice the amount of memory than needed because we want to avoid prefix operation (and added complexity of leaf indices)? Twice because there would be equal number of interior nodes as there would leaves.
Since
vector_leafis part of the model which will live on beyond the training phase, is compaction of the array worth considering?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I don't think it's important, I favor the simple approach. I'm assuming that this is not related to the performance regression, will investigate that more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering about the conclusion of this conversation, would array compaction be a worthy optimization in a further PR? (I doubt it would be worth it too much for the memory savings, but wanted to as anyway)