Repository: sktime/sktime

Issue:
[DOC] BaseForecaster score function uses MAPE instead of sMAPE as metric
#### Describe the issue linked to the documentation

The [score function](https://github.com/sktime/sktime/blob/ed4efe9e47fa998ad6b21d63b0ac58f932ab1b3f/sktime/forecasting/base/_base.py#L1234C1-L1234C1) of the BaseForecaster suggests the metric that is used for evaluation is sMAPE:

```
Returns

        -------

        score : float
            sMAPE loss of self.predict(fh, X) with respect to y_test.
```
            
The code calls the [mean_absolute_percentage_error function](https://github.com/sktime/sktime/blob/ed4efe9e47fa998ad6b21d63b0ac58f932ab1b3f/sktime/performance_metrics/forecasting/_functions.py#L1448) without any further parameters. 

```     
from sktime.performance_metrics.forecasting import (
            mean_absolute_percentage_error,
        )

        return mean_absolute_percentage_error(y, self.predict(fh, X))
``` 

But the default values of the [mean_absolute_percentage_error function ](https://github.com/sktime/sktime/blob/ed4efe9e47fa998ad6b21d63b0ac58f932ab1b3f/sktime/performance_metrics/forecasting/_functions.py#L1448) are: 

```
def mean_absolute_percentage_error(
    y_true,
    y_pred,
    horizon_weight=None,
    multioutput="uniform_average",
    symmetric=False,
    **kwargs,
):
```

and the doc states:

```
... 
    symmetric : bool, default=False
        Calculates symmetric version of metric if True.
...
```

Thus I conclude that instead of the sMAPE, MAPE is returned.

#### Suggest a potential alternative/fix

Either adjust the documentation such that it states that the MAPE metric is used or adjust the implementation by adding the symmetric parameter. I did not investigate if there are other places with the documentation error. ;) Many thanks!! 


Discussion:

Patch:
diff --git a/.all-contributorsrc b/.all-contributorsrc
index dc55cf738..9e60b703e 100644
--- a/.all-contributorsrc
+++ b/.all-contributorsrc
@@ -2228,6 +2228,15 @@
         "bug"
       ]
     },
+    {
+      "login": "Gigi1111",
+      "name": "Chung-Fan Tsai",
+      "avatar_url": "https://avatars.githubusercontent.com/Gigi1111",
+      "profile": "https://github.com/Gigi1111",
+      "contributions": [
+        "test"
+      ]
+    },
     {
       "login": "eyjo",
       "name": "Eyjólfur Sigurðsson",
@@ -2243,6 +2252,27 @@
       "name": "Julia Kraus",
       "avatar_url": "https://avatars.githubusercontent.com/julia-kraus",
       "profile": "https://github.com/julia-kraus",
+      "contributions": [
+        "doc",
+        "code",
+        "test"
+      ]
+    },
+    {
+      "login": "davidgilbertson",
+      "name": "David Gilbertson",
+      "avatar_url": "https://avatars.githubusercontent.com/u/4443482?v=4",
+      "profile": "https://github.com/davidgilbertson",
+      "contributions": [
+        "code",
+        "bug"
+      ]
+    },
+    {
+      "login": "MBristle",
+      "name": "Mirko Bristle",
+      "avatar_url": "https://avatars.githubusercontent.com/MBristle",
+      "profile": "https://github.com/MBristle",
       "contributions": [
         "doc"
       ]
diff --git a/docs/source/api_reference/forecasting.rst b/docs/source/api_reference/forecasting.rst
index 4e4dfbce6..d7e9148f0 100644
--- a/docs/source/api_reference/forecasting.rst
+++ b/docs/source/api_reference/forecasting.rst
@@ -428,6 +428,7 @@ Time series splitters can be used in both evaluation and tuning.
     SingleWindowSplitter
     SlidingWindowSplitter
     ExpandingWindowSplitter
+    ExpandingGreedySplitter
 
 .. autosummary::
     :toctree: auto_generated/
diff --git a/sktime/forecasting/base/_base.py b/sktime/forecasting/base/_base.py
index acd588550..0cb618146 100644
--- a/sktime/forecasting/base/_base.py
+++ b/sktime/forecasting/base/_base.py
@@ -1232,7 +1232,7 @@ class BaseForecaster(BaseEstimator):
         return y_res
 
     def score(self, y, X=None, fh=None):
-        """Scores forecast against ground truth, using MAPE.
+        """Scores forecast against ground truth, using MAPE (non-symmetric).
 
         Parameters
         ----------
@@ -1252,7 +1252,7 @@ class BaseForecaster(BaseEstimator):
         Returns
         -------
         score : float
-            sMAPE loss of self.predict(fh, X) with respect to y_test.
+            MAPE loss of self.predict(fh, X) with respect to y_test.
 
         See Also
         --------
@@ -1260,12 +1260,12 @@ class BaseForecaster(BaseEstimator):
         """
         # no input checks needed here, they will be performed
         # in predict and loss function
-        # symmetric=True is default for mean_absolute_percentage_error
         from sktime.performance_metrics.forecasting import (
             mean_absolute_percentage_error,
         )
 
-        return mean_absolute_percentage_error(y, self.predict(fh, X))
+        # specify non-symmetric explicitly as it changed in the past
+        return mean_absolute_percentage_error(y, self.predict(fh, X), symmetric=False)
 
     def get_fitted_params(self, deep=True):
         """Get fitted parameters.
diff --git a/sktime/forecasting/croston.py b/sktime/forecasting/croston.py
index 9bf9aaf11..9c2186c78 100644
--- a/sktime/forecasting/croston.py
+++ b/sktime/forecasting/croston.py
@@ -167,7 +167,10 @@ class Croston(BaseForecaster):
         -------
         params : dict or list of dict
         """
-        params1 = {}
-        params2 = {"smoothing": 0.42}
+        params = [
+            {},
+            {"smoothing": 0},
+            {"smoothing": 0.42},
+        ]
 
-        return [params1, params2]
+        return params
diff --git a/sktime/forecasting/exp_smoothing.py b/sktime/forecasting/exp_smoothing.py
index a9f4c09d2..62ee6b991 100644
--- a/sktime/forecasting/exp_smoothing.py
+++ b/sktime/forecasting/exp_smoothing.py
@@ -215,18 +215,32 @@ class ExponentialSmoothing(_StatsModelsAdapter):
             `MyClass(**params)` or `MyClass(**params[i])` creates a valid test instance.
             `create_test_instance` uses the first (or only) dictionary in `params
         """
-        params1 = {}
-        params2 = {
-            "trend": "mul",
-            "damped_trend": True,
-            "seasonal": "mul",
-            "sp": 2,
-            "use_boxcox": False,
-            "initialization_method": "heuristic",
-            "smoothing_level": 0.1,
-            "smoothing_trend": 0.1,
-            "damping_trend": 0.42,
-            "method": "least_squares",
-        }
-
-        return [params1, params2]
+        params = [
+            {},
+            {
+                "trend": "mul",
+                "damped_trend": True,
+                "seasonal": "mul",
+                "sp": 2,
+                "use_boxcox": False,
+                "initialization_method": "heuristic",
+                "smoothing_level": 0.1,
+                "smoothing_trend": 0.1,
+                "damping_trend": 0.42,
+                "method": "least_squares",
+            },
+            {
+                "trend": "add",
+                "damped_trend": False,
+                "seasonal": "add",
+                "sp": 2,
+                "use_boxcox": True,
+                "initialization_method": "estimated",
+                "smoothing_level": 0.3,
+                "smoothing_trend": 0.5,
+                "damping_trend": 0.28,
+                "method": "SLSQP",
+            },
+        ]
+
+        return params
diff --git a/sktime/forecasting/model_selection/__init__.py b/sktime/forecasting/model_selection/__init__.py
index 825bba210..c55f73960 100644
--- a/sktime/forecasting/model_selection/__init__.py
+++ b/sktime/forecasting/model_selection/__init__.py
@@ -9,6 +9,7 @@ __all__ = [
     "SingleWindowSplitter",
     "SlidingWindowSplitter",
     "temporal_train_test_split",
+    "ExpandingGreedySplitter",
     "ExpandingWindowSplitter",
     "TestPlusTrainSplitter",
     "ForecastingGridSearchCV",
@@ -18,6 +19,7 @@ __all__ = [
 
 from sktime.forecasting.model_selection._split import (
     CutoffSplitter,
+    ExpandingGreedySplitter,
     ExpandingWindowSplitter,
     SameLocSplitter,
     SingleWindowSplitter,
diff --git a/sktime/forecasting/model_selection/_split.py b/sktime/forecasting/model_selection/_split.py
index 5a058f5e1..5fb23b18e 100644
--- a/sktime/forecasting/model_selection/_split.py
+++ b/sktime/forecasting/model_selection/_split.py
@@ -3,6 +3,7 @@
 """Implement dataset splitting for model evaluation and selection."""
 
 __all__ = [
+    "ExpandingGreedySplitter",
     "ExpandingWindowSplitter",
     "SlidingWindowSplitter",
     "CutoffSplitter",
@@ -11,7 +12,14 @@ __all__ = [
     "temporal_train_test_split",
     "TestPlusTrainSplitter",
 ]
-__author__ = ["mloning", "kkoralturk", "khrapovs", "chillerobscuro", "fkiraly"]
+__author__ = [
+    "mloning",
+    "kkoralturk",
+    "khrapovs",
+    "chillerobscuro",
+    "fkiraly",
+    "davidgilbertson",
+]
 
 from typing import Iterator, Optional, Tuple, Union
 
@@ -1185,6 +1193,93 @@ class ExpandingWindowSplitter(BaseWindowSplitter):
         return self._split_windows_generic(expanding=True, **kwargs)
 
 
+class ExpandingGreedySplitter(BaseSplitter):
+    """Splitter that uses all available data.
+
+    Takes an integer `test_size` that defines the number of steps included in the
+    test set of each fold. The train set of each fold will contain all data before
+    the test set. If the data contains multiple instances, `test_size` is
+    _per instance_.
+
+    If no `step_length` is defined, the test sets (one for each fold) will be
+    adjacent, taken from the end of the dataset.
+
+    For example, with `test_size=7` and `folds=5`, the test sets in total will cover
+    the last 35 steps of the data with no overlap.
+
+    Parameters
+    ----------
+    test_size : int
+        The number of steps included in the test set of each fold.
+    folds : int, default = 5
+        The number of folds.
+    step_length : int, optional
+        The number of steps advanced for each fold. Defaults to `test_size`.
+
+    Examples
+    --------
+    >>> import numpy as np
+    >>> from sktime.forecasting.model_selection import ExpandingGreedySplitter
+
+    >>> ts = np.arange(10)
+    >>> splitter = ExpandingGreedySplitter(test_size=3, folds=2)
+    >>> list(splitter.split(ts))  # doctest: +SKIP
+    [
+        (array([0, 1, 2, 3]), array([4, 5, 6])),
+        (array([0, 1, 2, 3, 4, 5, 6]), array([7, 8, 9]))
+    ]
+    """
+
+    _tags = {"split_hierarchical": True}
+
+    def __init__(self, test_size: int, folds: int = 5, step_length: int = None):
+        super().__init__()
+        self.test_size = test_size
+        self.folds = folds
+        self.step_length = step_length
+        self.fh = np.arange(test_size) + 1
+
+    def _split(self, y: pd.Index) -> SPLIT_GENERATOR_TYPE:
+        if isinstance(y, pd.MultiIndex):
+            groups = pd.Series(index=y).groupby(y.names[:-1])
+            reverse_idx = groups.transform("size") - groups.cumcount() - 1
+        else:
+            reverse_idx = np.arange(len(y))[::-1]
+
+        step_length = self.step_length or self.test_size
+
+        for i in reversed(range(self.folds)):
+            tst_end = i * step_length
+            trn_end = tst_end + self.test_size
+            trn_indices = np.flatnonzero(reverse_idx >= trn_end)
+            tst_indices = np.flatnonzero(
+                (reverse_idx < trn_end) & (reverse_idx >= tst_end)
+            )
+            yield trn_indices, tst_indices
+
+    @classmethod
+    def get_test_params(cls, parameter_set="default"):
+        """Return testing parameter settings for the splitter.
+
+        Parameters
+        ----------
+        parameter_set : str, default="default"
+            Name of the set of test parameters to return, for use in tests. If no
+            special parameters are defined for a value, will return `"default"` set.
+
+        Returns
+        -------
+        params : dict or list of dict, default = {}
+            Parameters to create testing instances of the class
+            Each dict are parameters to construct an "interesting" test instance, i.e.,
+            `MyClass(**params)` or `MyClass(**params[i])` creates a valid test instance.
+            `create_test_instance` uses the first (or only) dictionary in `params`
+        """
+        params1 = {"test_size": 1}
+        params2 = {"test_size": 3, "folds": 2, "step_length": 2}
+        return [params1, params2]
+
+
 class SingleWindowSplitter(BaseSplitter):
     r"""Single window splitter.
 


Test patch:
diff --git a/sktime/forecasting/model_selection/tests/test_split.py b/sktime/forecasting/model_selection/tests/test_split.py
index 3c9ecba60..887551003 100644
--- a/sktime/forecasting/model_selection/tests/test_split.py
+++ b/sktime/forecasting/model_selection/tests/test_split.py
@@ -11,6 +11,7 @@ from sktime.datatypes._utilities import get_cutoff
 from sktime.forecasting.base import ForecastingHorizon
 from sktime.forecasting.model_selection import (
     CutoffSplitter,
+    ExpandingGreedySplitter,
     ExpandingWindowSplitter,
     SameLocSplitter,
     SingleWindowSplitter,
@@ -325,6 +326,92 @@ def test_sliding_window_splitter_start_with_empty_window(
             )
 
 
+def test_expanding_greedy_splitter_lengths():
+    """Test that ExpandingGreedySplitter returns the correct lengths."""
+    y = np.arange(10)
+    cv = ExpandingGreedySplitter(test_size=2, folds=3)
+
+    lengths = [(len(trn), len(tst)) for trn, tst in cv.split_series(y)]
+    assert lengths == [(4, 2), (6, 2), (8, 2)]
+
+
+def test_expanding_greedy_splitter_dates():
+    """Test that ExpandingGreedySplitter splits on dates correctly."""
+    ts = _make_series(index_type="period")
+    first_date = ts.index[0]
+    last_date = ts.index[-1]
+    cv = ExpandingGreedySplitter(test_size=2, folds=3)
+
+    train_starts = []
+    train_ends = []
+    test_starts = []
+    test_ends = []
+
+    for trn, tst in cv.split_series(ts):
+        train_starts.append(trn.index[0])
+        train_ends.append(trn.index[-1])
+        test_starts.append(tst.index[0])
+        test_ends.append(tst.index[-1])
+
+    assert train_starts == [first_date, first_date, first_date]
+    assert train_ends == [last_date - 6, last_date - 4, last_date - 2]
+    assert test_starts == [last_date - 5, last_date - 3, last_date - 1]
+    assert test_ends == [last_date - 4, last_date - 2, last_date]
+
+
+def test_expanding_greedy_splitter_hierarchy():
+    """Test that ExpandingGreedySplitter handles uneven hierarchical data."""
+    y_panel = _make_hierarchical(
+        hierarchy_levels=(2, 4),
+        max_timepoints=20,
+        min_timepoints=10,
+        same_cutoff=False,
+    )
+
+    cv = ExpandingGreedySplitter(test_size=2, folds=3)
+
+    # Since same_cutoff=False above, we will have a potentially different end date for
+    # each instance in the hierarchy. Below we test that the final fold ends at the
+    # correct end date for each instance
+    last_test_fold = list(cv.split_series(y_panel))[-1][1]
+
+    last_test_dates_actual = (
+        last_test_fold.reset_index(-1)
+        .groupby(last_test_fold.index.names[:-1])
+        .tail(1)["time"]
+    )
+    last_test_dates_expected = (
+        y_panel.reset_index(-1).groupby(y_panel.index.names[:-1]).tail(1)["time"]
+    )
+
+    assert last_test_dates_actual.eq(last_test_dates_expected).all()
+
+
+def test_expanding_greedy_splitter_consecutive():
+    """Test that ExpandingGreedySplitter results in consecutive test periods."""
+    y_panel = _make_hierarchical(
+        hierarchy_levels=(2, 4),
+        max_timepoints=20,
+        min_timepoints=10,
+        same_cutoff=False,
+    )
+
+    cv = ExpandingGreedySplitter(test_size=2, folds=3)
+
+    test_dfs = [tst for _, tst in cv.split_series(y_panel)]
+
+    combined_test_df = pd.concat(test_dfs).sort_index()
+
+    # We check the .diff(). Equal diffs = uniformly spaced timestamps
+    has_consecutive_index = (
+        combined_test_df.reset_index(-1)
+        .groupby(combined_test_df.index.names[:-1])["time"]
+        .agg(lambda s: len(s.diff().value_counts()) == 1)
+    )
+
+    assert has_consecutive_index.all()
+
+
 @pytest.mark.parametrize("y", TEST_YS)
 @pytest.mark.parametrize("fh", [*TEST_FHS, *TEST_FHS_TIMEDELTA])
 @pytest.mark.parametrize("step_length", TEST_STEP_LENGTHS)