Tree MAE fix to ensure sample_weights are used during impurity calculation. #11461
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tree MAE is not considering sample_weights when calculating impurity! In the proposed fix, you will see I have multiplied by the sample weight *after* applying the absolute to the difference (not before). This is in line with the consensus / discussion found here, where negative sample weights are considered: scikit-learn#3774 (and also because during initialisation, self.weighted_n_node_samples is a summation of the sample weights with no "absolute" applied (this is used in the impurity divisor)).
|
(I've just noticed upload incompatibilities (whitespace) introduced due to Windows-> Unix, I'll resubmit later and also check through the validation issues as seen in last commit) |
|
thanks!
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the proposed fix, you will see I have multiplied by the sample weight after applying the absolute to the difference (not before). This is in line with the consensus / discussion found here, where negative sample weights are considered: #3774 (and also because during initialisation, self.weighted_n_node_samples is a summation of the sample weights with no "absolute" applied (this is used in the impurity division calc)).
Reference Issues/PRs
Fixes: #11460 (Tree MAE is not considering sample_weights when calculating impurity!)
What does this implement/fix? Explain your changes.
Any other comments?