[ENH] fitted parameter forwarding utility, forward statsforecast estimators' fitted parameters

[fkiraly]

This PR adds a fitted parameter forwarding utility which is useful for adapter classes. It copies fitted parameters in the sense of the sklearn convention that these end in underscore from any object - usually the delegate, nthe adapter case - to a target object - usually self.

Further, the utility is added to adapters as follows:

• adds fitted parameter forwarding in _GeneralisedStatsforecastAdapter. This fixes [ENH] expose fitted params for statsforecast forecasters via get_fitted_params #5720
• replaces the ad-hoc code in _PytsAdapter with the utility
• replaces the ad-hoc code in _TslearnAdapter with the utility

[yarnabrina]

Not a review, just a clarification question

This PR adds a fitted parameter forwarding utility which is useful for adapter classes. It copies fitted parameters in the sense of the sklearn convention that these end in underscore from any object - usually the delegate, nthe adapter case - to a target object - usually self.

Based on the docstrings in extension templates, I had the understanding that existing base class already implements this, i.e. if I create self.abc_ in an estimator, abc will be available as a key of the fitted parameters. Is it a complete misconception?

[fkiraly]

Based on the docstrings in extension templates, I had the understanding that existing base class already implements this, i.e. if I create self.abc_ in an estimator, abc will be available as a key of the fitted parameters. Is it a complete misconception?

That is correct, but that will not help with adapters, as there the fitted parameters are nested into a delegate, e.g., self._delegate_estimator.aic_.

So the default get_fitted_params will not discover aic_.

There are multiple solutions here, out of these I think the one in this PR is best:

• solution 1: name the delegate estimatror delegate_estimator_, that will expose the parameter as delegate_estimator__aic. This is confusing to the user, as the nested notation is usually only used for components the user themselves set.
• solution 2: override _get_fitted_params. This is problematic since sklearn expects the attrs to be in self, so we would depart from interface consistency.
• solution 3: this. I see no issue with copying/forwarding, given that the _delegate_estimator is intended to be "under the hood" by design of adapters.

[yarnabrina]

Thanks for the explanation, it makes sense now. I understand what you did here, but I have some hestation to make every single attribute ending with underscore as fitted parameters. These delegated estimators most likely (if not always) non sktime/sklearn package objects, and not all of our dependencies are sklearn compatible. They may follow different naming conventions and exposing all of them can mean exposing some internal properties as well as too many things to expose for end users.

It's just a precautionary totally subjective hesitation, not a strong objection.

[fkiraly]

@yarnabrina, could you explain, what is your caution against, specifically? I cannot see how this could cause excpetions, because:

• the getattr is wrapped in a try/excpet safety loop. So, even if some parameters are properties raising errors, this is caught.
• The default is "do not overwrite self", so no core attributes ever get overwritten.
• at least for pyts and tslearn it should be fine, these packages also follow the sklearn interface standards with fitted parameters.

[yarnabrina]

I cannot see how this could cause excpetions

I agree completely, that's not my worry.

My caution is that this will expose every single attributes from underlying packages ending with underscore. I think this may be too many to be useful for end users, and few of them may not really fit well into the idea of "fitted parameters".

As an example, if you try the statsforecast adapter, it'll expose model_, which seems to have a complete different schema for different models and sometimes contain even residuals. I am not sure if these are expected by sktime users as outputs of this method.

It's just a precautionary totally subjective hesitation

Please consider this with as much emphasis on subjective as possible.

[fkiraly]

My caution is that this will expose every single attributes from underlying packages ending with underscore.

Can you confirm, your worry is "too many attributes that are not useful"?

I am not sure if these are expected by sktime users as outputs of this method.

Hm, I thought that was exactly what @jiayuanteng wanted though - perhaps they could confirm if that is the case or not?

[yarnabrina]

Can you confirm, your worry is "too many attributes that are not useful"?

Yes, confirmed.

[fkiraly]

Is that happening when you try out this PR with statsforecast estimators?

[jiayuanteng]

Sorry if I was not clear earlier. What I would like to have is to get fitted parameters (estimated coefficients) from the best model.

[fkiraly]

Is this PR giving you access to the parameters? My question was about the actual get_fitted_params return, which you said was empty.

@yarnabrina, would you agree that any dict is better than an empty dict?

[yarnabrina]

@yarnabrina, would you agree that any dict is better than an empty dict?

Yes, and I'd agree to merge this as default settings. But I'll prefer if adapter wise we can find better options customised for underlying soft dependency.

For that, I'd prefer to wait for a day or two for a possible update from Nixtla if they can offer a better suggestion, at least for statsforecast. Are you planning to release this for 0.29.0 today?

[fkiraly]

no, I was not - 0.29.0 only has maintenance/deprecation content above 0.28.1.