Support for Categorical features in CalculateFeatureContribution of LightGBM #5018
Conversation
| foreach (var index in CategoricalSplitFeatures[node]) | ||
| { | ||
| float fv = GetFeatureValue(src.GetItemOrDefault(index), node); | ||
| if (fv > 0.0f) | ||
| { | ||
| newNode = GtChild[node]; | ||
| otherWay = LteChild[node]; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // What if we went the other way? | ||
| var ghostLeaf = GetLeafFrom(in src, otherWay); | ||
| var ghostOutput = GetOutput(ghostLeaf); | ||
|
|
||
| // If the ghost got a smaller output, the contribution of the categorical features is positive, so | ||
| // the contribution is true minus ghost. | ||
| foreach(var ifeat in CategoricalSplitFeatures[node]) | ||
| contributions.AddFeature(ifeat, (float)(trueOutput - ghostOutput)); |
There was a problem hiding this comment.
Code-wise, I think this is the correct way to find which features are involved in the categorical split of this node (as it is done in a similar way here) And I tried to make this analogous to how feature contribution is already being calculated for non-categorical features (here).
But I don't know if this is the "mathematically correct" way of calculating feature contributions for categorical features. I can think about a couple of alternatives to this, but I wouldn't know which one to choose.
There was a problem hiding this comment.
So I've updated this. I am still not sure the updated version is correct. But I think it's the closest to how feature contribution is calculated for numerical feature splits, and considering that for other other cases (FastTree, Gam, etc...) categorical features are treated the same as any other feature (ignoring the fact they're categorical) when calculating feature contribution.
|
Does this also affect #3766 ? |
|
Hi, @rauhs. This PR here won't affect issue #3766. Nonetheless, having a quick look at that issue, it seems it is similar in nature to issue #3272 which is fixed in this PR. So fixing this issue here won't fix that other issue. I've left some comments on that issue explaining what needs to be done to solve it. Thanks for pointing to that issue! In reply to: 612639332 [](ancestors = 612639332) |
| @@ -57,7 +57,7 @@ public abstract class RegressionTreeBase | |||
| /// (2) the categorical features indexed by <see cref="GetCategoricalCategoricalSplitFeatureRangeAt(int)"/>'s | |||
| /// returned value with nodeIndex=i is NOT a sub-set of <see cref="GetCategoricalSplitFeaturesAt(int)"/> with | |||
| /// nodeIndex=i. | |||
| /// Note that the case (1) happens only when <see cref="CategoricalSplitFlags"/>[i] is true and otherwise (2) | |||
| /// Note that the case (1) happens only when <see cref="CategoricalSplitFlags"/>[i] is false and otherwise (2) | |||
There was a problem hiding this comment.
It looks to me this doc was wrong, as it's inconsistent with what these other lines below say:
machinelearning/src/Microsoft.ML.FastTree/RegressionTree.cs
Lines 76 to 80 in 8660ecc
machinelearning/src/Microsoft.ML.FastTree/RegressionTree.cs
Lines 100 to 106 in 8660ecc
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ghost
locked as resolved and limited conversation to collaborators
Fixes #3272
As explained here,
CalculateFeatureContributionwould throw an exception when used on LightGBM models that hadUseCategoricalSplitenabled, because there was no support to calculate feature contribution for categorical features. Here I add that support, and one test to replicate the original scenario were an exception was thrown.