[BUG] MeanAbsoluteError does not leverage horizon_weight

[kdekker-private]

Describe the bug
The performance metric MeanAbsoluteError does not take into account horizon_weight when calling the class instance.
All other scikit-learn based metrics do.

To Reproduce

import numpy as np
from sktime.performance_metrics.forecasting import MeanAbsoluteError
y_true = np.array([3, -0.5, 2, 7, 2])
y_pred = np.array([2.5, 0.0, 2, 8, 1.25])
mae = MeanAbsoluteError()
mae(y_true, y_pred) != mae(y_true, y_pred, horizon_weight=np.array([0.1,0.2,0.1,0.3,0.4]))

Expected behavior
The boolean statement at the end should be true.

Additional context

Versions
0.28.0

[fkiraly]

All other scikit-learn based metrics do.

Can you give an example of a class that does?

I was not aware of the argument being used by the classes, and wonder why it is present in the functions and not the classes.

[kdekker-private]

Metrics class MeanSquaredError does.

Example:

import numpy as np
from sktime.performance_metrics.forecasting import MeanSquaredError
y_true = np.array([3, -0.5, 2, 7, 2])
y_pred = np.array([2.5, 0.0, 2, 8, 1.25])
mse = MeanSquaredError()
mse(y_true, y_pred) != mse(y_true, y_pred, horizon_weight=np.array([0.1,0.2,0.1,0.3,0.4]))

[fkiraly]

Ok, this has puzzled me quite a bit but I now understand what is going on.

The horizon_weight parameter is an undocumented and untested yet public interface point of metrics classes. It is accessible because evaluate forwards kwargs, and some metrics relying on functions forwarded the weight param to the function (not sklearn but sktime functions).

Now, we have recently began to improve runtime performance of metrics gradually, and that refactor inadvertently removed this undocumented interface point, namely MeanAbsoluteError in 0.28.0, and MeanSquaredError in 0.29.0. Funnily, it was @marrov's request for performance improvements which caused this indirectly. Tests also did not discover this degradation of public behaviour, because it was neither known to devs nor tested.

FYI @sktime/core-developers, this is a textbook example of Hyrum's law 😁
https://www.hyrumslaw.com/

Either way, I think we should be "nice" about this, proposed resolution:

weights are actually a nice feature to have for metrics, and it does not cost much to add it to all metrics, and to add it back to the refactored metrics. For compatibility with sklearn, we should name the arg sample_weight, but redirect horizon_weight to it if passed, with a user warning.

[fkiraly]

Question out of interest, @kdekker-private, how did you get the idea to use the horizon_weight arg in a class? Is there a tutorial or other code that does that?