RFC SLEP006: verbose vs non-verbose declaration in meta-estimator

[adrinjalali]

As the proposal and the implementation of meta-estimator routing (SLEP006) stands, if the user wants to use sample_weight, they need to be quite verbose in how they declare the estimators. Taking AdaBoostClassifier as an example, and imagining if AdaBoostClassifier would use the sub-estimator's score method, the user would have to write:

est = (
    AdaBoostClassifier(LogisticRegression().set_fit_request(sample_weight=True)
    .set_score_request(sample_weight=True))
    .fit(X, y, sample_weight=sw)
)

which is quite more verbose than the current code users need to write:

est = AdaBoostClassifier(LogisticRegression()).fit(X, y, sample_weight=sw)

There have been concerns about making users write quite verbose code in cases where the current pattern seems quite reasonable.

Without changing everything related to SLEP006, there are three paths we can take:

Option 1: Helper function

We can introduce helper functions to make the above code simpler. For instance, a weighted function could request sample_weight on all methods which accept sample_weight for a given estimator. Then the above code would look like:

est = AdaBoostClassifier(weighted(LogisticRegression())).fit(X, y, sample_weight=sw)

and if the sub-estimator is a pipeline:

est = AdaBoostClassifier(
    make_pipeline(weighted(StandardScaler()), weighted(LogisticRegression())))
).fit(X, y, sample_weight=sw)

Implementing weighted for a Pipeline (or other meta-estimators) would be tricky since set_fit_request is only available for consumers and not non-consumer routers; therefore the user needs to repeat the weighted call for all sub-estimators.

Option 2: Different meta-estimators

Have two classes of meta-estimators (or routers to be specific).

In this scenario, we divide meta-estimators into two classes, simple and complex. Simple routers are the ones which simply forward **kwargs to sub-estimators, and by default the assume sub-estimators have requested those metadata. This simplifies the users' code and makes the existing code for simple meta-estimators to keep working, but it raises a few issues.

First is that there will be two classes of meta-estimators, and the user would need to know which estimator is of which class. It's also not clear what we should do if the user explicitly sets request values for metadata (we can probably respect those if present).

Another issue is that if a meta-estimator changes behavior, it needs to become a complex meta-estimator if we want to keep backward compatibility for it. This doesn't seem like a good pattern.

Option 3: Keep as is

Do nothing, things are as is.

I'm in favor or option 1 because:

• with the helper function the user code doesn't look too verbose
• using metadata is not a beginner kinda thing and therefore this API is not hampering beginners' experience with the library
• it keeps consistency among meta-estimators/consumers

xref: #22986 (review)

[jnothman]

The increased verbosity in simple cases indeed makes me a bit sad.

AdaBoostClassifier is a bit unusual because the meta-estimator needs to send sample_weight to the base estimator, regardless of whether weights are passed in. As such, AdaBoostClassifier might be seen as a consumer of the sample_weight which it then modifies. What it is doing with relation to its base estimator and sample_weight is certainly not routing, but rather producing. Thus a consumer should only have to set_fit_request(sample_weight=True) to be passed into AdaBoostClassifier in the case that it is wrapped in a Pipeline or similar... which makes sense! How to explain this apparent "exception" in the documentation? I'm not sure...

[jnothman]

The cases where, to me, we're increasing verbosity unnecessarily are cases where the metaestimator always has one destination. For example, MultiOutputRegressor. This is eligible for the "simple routers" treatment, which is simple precisely because it's not routing, just passing.

[adrinjalali]

The cases where, to me, we're increasing verbosity unnecessarily are cases where the metaestimator always has one destination. For example, MultiOutputRegressor. This is eligible for the "simple routers" treatment, which is simple precisely because it's not routing, just passing.

Can you elaborate a bit? The sub-estimator of a MultiOutputRegressor can always be a pipeline.

In general, we can't check for signature as we're doing now. Signature checks are a hack which kinda work, but are quite error prone.

What you're suggesting, to me sounds like option (2) in the post above, where you'd like to have two classes of routers, simple and complex ones. Simple ones always route everything and work like a [network] bridge, complex ones do some sort of routing.

[jnothman]

The sub-estimator of a MultiOutputRegressor can always be a pipeline.

But then we can still pass whatever args are given to MultiOutputRegressor.fit to is base estimator. If it's a Pipeline it will know how to route those args.

Not sure if that's the elaboration you sought

[adrinjalali]

So with that, you're arguing for option (2) in my top post. I don't mind it, and we do have the tools to support it. As mentioned above, I just worry about the education part, since it creates two classes of meta-estimators. But I'm happy to go down that route if you think it makes sense.

ping @scikit-learn/core-devs

[lorentzenchr]

What about a 4th option: Not a helper function, but a helper method

est = AdaBoostClassifier(LogisticRegression().set_all_request_sample_weight()).fit(X, y, sample_weight=sw)

Or set_all_request(sample_weight=True).
Name to be discussed.

[jnothman]

I like the readability of weighted, but I think there's too much implicit in it. Hard for the reader to know that scoring is weighted. I'd prefer explicit there. I can imagine many users defining weighted at the top of their scripts to avoid .set_fit_request(sample_weight=True), but I'm not sure something based on sniffing places that sample_weight may be requested is a good idea.

[jnothman]

Having different kinds of metaestimators is okay, but we need to be clear on what qualifies... Otherwise okay to leave as is for now.

[adrinjalali]

Another solution would be something like:

with sklearn.config_context(sample_weight_requested=True):
	est = AdaBoostClassifier(LogisticRegression()).fit(X, y, sample_weight=sw)

cc @betatim