FIX Draw indices using sample_weight in Forest #31529
Conversation
sklearn/ensemble/_forest.py
Outdated
| if sample_weight is None: | ||
| sample_weight = np.ones(n_samples) | ||
| normalized_sample_weight = sample_weight / np.sum(sample_weight) | ||
| sample_indices = random_instance.choice( | ||
| n_samples, n_samples_bootstrap, replace=True, p=normalized_sample_weight | ||
| ) |
There was a problem hiding this comment.
I hesitate between two options for dealing with the sample_weight=None case.
- Convert to all ones.
if sample_weight is None:
sample_weight = np.ones(n_samples)
normalized_sample_weight = sample_weight / np.sum(sample_weight)
sample_indices = random_instance.choice(
n_samples, n_samples_bootstrap, replace=True, p=normalized_sample_weight
)- Use the old code path when
sample_weight=None
if sample_weight is None:
sample_indices = random_instance.randint(
0, n_samples, n_samples_bootstrap, dtype=np.int32
)
else:
normalized_sample_weight = sample_weight / np.sum(sample_weight)
sample_indices = random_instance.choice(
n_samples,
n_samples_bootstrap,
replace=True,
p=normalized_sample_weight,
)The benefit of 2. is that the code is backward compatible when sample_weight=None, this PR and main give the exact same fit for a given random_state.
The benefit of 1. is that sample_weight=None and sample_weight=np.ones(n_samples) give the exact same fit for a given random_state.
Using 1. test_set_estimator_drop, test_rfe_features_importance or test_forest_classifier_oob fail. These tests do not use sample_weight.
Using 2. test_class_weights fails, because it checks that no or all ones sample_weight give the same results.
| # NOTE: "balanced_subsample" option is ignored, treated as "balanced" | ||
| class_weight = self.class_weight | ||
| if class_weight == "balanced_subsample": | ||
| class_weight = "balanced" | ||
| expanded_class_weight = compute_sample_weight(class_weight, y_original) |
There was a problem hiding this comment.
Here I choose to simply ignore the "balanced_subsample" option and treat it as the "balanced" case.
|
The forest estimators now pass the statistical repeated/weighted equivalence test, for example |
Part of #16298. Similar to #31414 (Bagging estimators) but for Forest estimators.
What does this implement/fix? Explain your changes.
When subsampling is activated (
bootstrap=True),sample_weightare now used as probabilities to draw the indices. Forest estimators then pass the statistical repeated/weighted equivalence test.Comments
This PR does not fix Forest estimators when
bootstrap=False(no subsampling).sample_weightare still passed to the decision trees. Forest estimators then fail the statistical repeated/weighted equivalence test because the individual treesalso fail this test (probably because of tied splits in decision trees #23728).
TODO
sample_weight=Nonecasemax_samplesas done in FIX Draw indices using sample_weight in Bagging #31414balanced_subsampleoption