Fix/smape (#2984)

* Eliminated spurious error on sape/smape when pred and actual are 0

* Removed test for s(m)ape value error when pred and acutal are zero

Test for 0 s(m)ape for identical serieses including zeros

* Removed identical lines in test_ape_zero

* removed unnecessary commented-out code

* updated smape docstring

* Updated changelog

* formatting

* Update darts/metrics/metrics.py

typo

Co-authored-by: Dustin Brunner <[email protected]>

* Clarify docstrings as suggested

Co-authored-by: Dustin Brunner <[email protected]>

* lint

* Fixed docstring

* Fixed updated docstring

* Formatting

* Fixed doc build?

* minor updates

* update changelog

---------

Co-authored-by: eschibli <[email protected]>
Co-authored-by: Dustin Brunner <[email protected]>
Co-authored-by: dennisbader <[email protected]>

Author: 4 people

CHANGELOG.md

@@ -13,6 +13,8 @@ but cannot always guarantee backwards compatibility. Changes that may **break co
 
 **Fixed**
 
+- Updated s(m)ape to not raise a ValueError when actuals and predictions are zero for the same timestep. [#2984](https://github.com/unit8co/darts/pull/2984) by [eschibli](https://github.com/eschibli).
+
 **Dependencies**
 
 - We set an upper version cap on `pandas<3.0.0` until we officially support it. [#2995](https://github.com/unit8co/darts/pull/2995) by [Dennis Bader](https://github.com/dennisbader).

darts/metrics/metrics.py

@@ -1744,8 +1744,8 @@ def sape(
     .. math::
         200 \\cdot \\frac{\\left| y_t - \\hat{y}_t \\right|}{\\left| y_t \\right| + \\left| \\hat{y}_t \\right|}
 
-    Note that it will raise a `ValueError` if :math:`\\left| y_t \\right| + \\left| \\hat{y}_t \\right| = 0` for some
-    :math:`t`. Consider using the Absolute Scaled Error (:func:`~darts.metrics.metrics.ase`)  in these cases.
+    When :math:`\\left| y_t \\right| + \\left| \\hat{y}_t \\right| = 0` for some :math:`t` (i.e., both actual and
+    prediction are zero), the error for that time step is defined as 0.
 
     If :math:`\\hat{y}_t` are stochastic (contains several samples) or quantile predictions, use parameter `q` to
     specify on which quantile(s) to compute the metric on. By default, it uses the median 0.5 quantile
@@ -1785,11 +1785,6 @@ def sape(
     verbose
         Optionally, whether to print operations progress.
 
-    Raises
-    ------
-    ValueError
-        If `actual_series` and `pred_series` contain some zeros at the same time index.
-
     Returns
     -------
     float
@@ -1822,14 +1817,14 @@ def sape(
         remove_nan_union=True,
         q=q,
     )
-    if not np.logical_or(y_true != 0, y_pred != 0).all():
-        raise_log(
-            ValueError(
-                "`actual_series` must be strictly positive to compute the sMAPE."
-            ),
-            logger=logger,
-        )
-    return 200.0 * np.abs(y_true - y_pred) / (np.abs(y_true) + np.abs(y_pred))
+    numerator = 200 * np.abs(y_true - y_pred)
+    denominator = np.abs(y_true) + np.abs(y_pred)
+    return np.divide(
+        numerator,
+        denominator,
+        out=np.zeros_like(numerator, dtype=y_true.dtype),
+        where=denominator != 0,
+    )
 
 
 @multi_ts_support
@@ -1854,9 +1849,8 @@ def smape(
         200 \\cdot \\frac{1}{T}
         \\sum_{t=1}^{T}{\\frac{\\left| y_t - \\hat{y}_t \\right|}{\\left| y_t \\right| + \\left| \\hat{y}_t \\right|} }
 
-    Note that it will raise a `ValueError` if :math:`\\left| y_t \\right| + \\left| \\hat{y}_t \\right| = 0`
-    for some :math:`t`. Consider using the Mean Absolute Scaled Error (:func:`~darts.metrics.metrics.mase`) in these
-    cases.
+    When :math:`\\left| y_t \\right| + \\left| \\hat{y}_t \\right| = 0` for some :math:`t` (i.e., both actual and
+    prediction are zero), the error for that time step is 0.
 
     If :math:`\\hat{y}_t` are stochastic (contains several samples) or quantile predictions, use parameter `q` to
     specify on which quantile(s) to compute the metric on. By default, it uses the median 0.5 quantile
@@ -1891,11 +1885,6 @@ def smape(
     verbose
         Optionally, whether to print operations progress.
 
-    Raises
-    ------
-    ValueError
-        If the `actual_series` and the `pred_series` contain some zeros at the same time index.
-
     Returns
     -------
     float

darts/tests/metrics/test_metrics.py

@@ -207,25 +207,35 @@ class TestMetrics:
         "metric",
         [
             metrics.ape,
-            metrics.sape,
             metrics.mape,
-            metrics.smape,
         ],
     )
     def test_ape_zero(self, metric):
         with pytest.raises(ValueError):
             metric(self.series1, self.series1)
 
-        with pytest.raises(ValueError):
-            metric(self.series1, self.series1)
-
     def test_ope_zero(self):
         with pytest.raises(ValueError):
             metrics.ope(
                 self.series1 - self.series1.to_series().mean(),
                 self.series1 - self.series1.to_series().mean(),
             )
 
+    @pytest.mark.parametrize(
+        "metric",
+        [
+            metrics.sape,
+            metrics.smape,
+        ],
+    )
+    def test_sape_zero_denom(self, metric):
+        assert np.allclose(metric(self.series0, self.series0), 0.0), (
+            "Expected SAPE to be 0.0 when both series are identical"
+        )
+        assert np.allclose(metric(self.series1, self.series1), 0.0), (
+            "Expected SAPE to be 0.0 when both series are identical"
+        )
+
     @pytest.mark.parametrize(
         "config",
         [