FIX Draw indices using sample_weight in Bagging #31414
Conversation
|
Idem for |
|
However it fails (as expected) for
|
Could you please document this known limitation, both in the docstring of the Something like: "Note that the expected frequency semantics for the
|
There was a problem hiding this comment.
Here is a pass of review. Could you please add a non-regression test using a small dataset with specifically engineered weights? For instance, you could have a dataset with 100 datapoints, with 98 data points with a null weight, 1 data point, with a weight of 1 and 1 with a weight of 2:
X = np.arange(100).reshape(-1, 1)
y = (X < 99).astype(np.int32)
sample_weight = np.zeros(shape=X.shape[0])
sample_weight[0] = 1
sample_weight[-1] = 2Then you could fit a BaggingRegressor and a BaggleClassifier with a fake test estimator that just records the values passed as X, y and sample_weight as fitted attribute to be able to write assertions in the test.
Ideally this test should pass both with metadata routing enabled and disabled.
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
| @@ -0,0 +1,5 @@ | ||
| - :class:`ensemble.BaggingClassfier`, :class:`ensemble.BaggingRegressor` | ||
| and :class:`ensemble.IsolationForest` now use `sample_weight` to draw | ||
| the samples instead of forwarding them multiplied with a uniformly sampling |
There was a problem hiding this comment.
Sorry, I made a typo in my past suggestion:
| the samples instead of forwarding them multiplied with a uniformly sampling | |
| the samples instead of forwarding them multiplied by a uniformly sampled |
Part of #16298 and alternative to #31165.
What does this implement/fix? Explain your changes.
In Bagging estimators,
sample_weightis now used to draw the samples and no longer forwarded to the underlying estimators. Bagging estimators now pass the statistical repeated/weighted equivalence test whenbootstrap=True(the default, ie draw with replacement).Compared to #31165, it better decouples two different usages of
sample_weight:sample_weightinbagging_estimator.fitare used as probabilities to draw the indices/rowssample_weightinbase_estimator.fitare used to represent the indices (more memory efficient than indexing), this is possible only ifbase_estimator.fitsupportssample_weight(through metadata routing or natively).#31165 introduced a new
sampling_strategyargument to choose indexing/weighting for row sampling, but it would be better to do this in a dedicated follow up PR.cc @ogrisel @GaetandeCast