Skip to content

fix: extend zero_division parameter to percentage and range-based metrics#3122

Open
mahimn01 wants to merge 1 commit into
unit8co:masterfrom
mahimn01:fix/metrics-zero-division-pct
Open

fix: extend zero_division parameter to percentage and range-based metrics#3122
mahimn01 wants to merge 1 commit into
unit8co:masterfrom
mahimn01:fix/metrics-zero-division-pct

Conversation

@mahimn01
Copy link
Copy Markdown

Checklist before merging this PR:

  • Mentioned all issues that this PR fixes or addresses.
  • Summarized the updates of this PR under Summary.
  • Added an entry under Unreleased in the Changelog.

Mirrors the design of #3059 for the percentage / range-based metric family. Adopts the breaking-change 🔴 marker convention discussed in #3080.

Summary

Percentage and range-based metrics (wmape, ope, arre, marre, coefficient_of_variation) previously either raised a hard ValueError or silently returned nan/inf when their denominator was zero. This made batch evaluation pipelines brittle for constant or all-zero components.

This PR extends the zero_division parameter pattern introduced in #3059 to these five metrics:

  • "warn" (default) — returns np.nan and emits a warning.
  • "raise" — preserves the legacy ValueError.

A new _safe_pct_divide helper sits next to _safe_scaled_divide in darts/metrics/utils.py. The two differ only in fill semantics: percentage metrics multiply the ratio by 100, so a 1.0 fill for the 0/0 case (the scaled-metric "on par with naive") would surface as 100 % error and be misleading — hence np.nan instead.

Two adjacent bugs are fixed in the same change:

  • ope previously checked sum > 0 and rejected actual_series with a strictly negative sum (e.g. financial return series). The check is now sum != 0 via the helper.
  • wmape's docstring claimed ValueError if actual_series contains some zeros, but the implementation divides by sum(|y_true|) and only the all-zero case ever triggered any path. Docstring corrected.

Behavior change

Input shape Behavior
Healthy inputs (non-zero denominator) Unchanged — all 455 existing tests pass without modification
Zero denominator + default "warn" Returns np.nan and logs a warning (previously: silent NaN/inf, or hard ValueError)
Zero denominator + "raise" Raises ValueError (legacy behavior preserved as opt-in)
Negative-sum actual_series in ope Now works correctly (previously: incorrectly raised)

Per the discussion in #3080, the CHANGELOG entry for the parameter addition carries the breaking-change 🔴 marker because the default behavior flips from raising to warning.

Test plan

  • New parametrized test_pct_metrics_zero_division covering all 5 metrics: warn-mode returns NaN with warning, raise-mode raises ValueError, invalid string rejected, healthy inputs unaffected.
  • New test_ope_accepts_negative_sum verifying the sum != 0 fix using a series with a strictly negative sum.
  • Updated existing test_ope_zero and test_arre to opt into legacy behavior with zero_division="raise".
  • uv run pytest darts/tests/metrics/test_metrics.py461 passed (was 455, +6 new).
  • uv run pre-commit run --files <changed> → all hooks pass (ruff, ruff-format, trailing whitespace, end-of-file).

Other Information

Files changed (+208/-31):

  • darts/metrics/utils.py_safe_pct_divide helper
  • darts/metrics/metrics.py — 5 metrics extended
  • darts/tests/metrics/test_metrics.py — 6 new tests + 2 legacy-test updates
  • CHANGELOG.md — entries under **Fixed** (the bug fixes) and **Improved** (the new parameter, with 🔴 marker)

…rics

Percentage and range-based metrics (`wmape`, `ope`, `arre`, `marre`,
`coefficient_of_variation`) previously either raised a hard `ValueError`
or silently returned `nan`/`inf` when their denominator was zero. This
made batch evaluation pipelines brittle for constant or all-zero
components.

Mirrors the `zero_division` design introduced in unit8co#3059 for the scaled
error family: `"warn"` (default) returns `np.nan` and emits a warning,
`"raise"` preserves the legacy `ValueError`. A new `_safe_pct_divide`
helper sits next to `_safe_scaled_divide`; the two differ only in fill
semantics — percentage metrics multiply the ratio by 100 so a `1.0`
fill for the 0/0 case (the scaled-metric "on par with naive") would
surface as `100 %` error and be misleading, hence `np.nan` instead.

Two adjacent bugs surface and are fixed in the same change:
* `ope` previously checked `sum > 0` and rejected `actual_series` with
  a strictly negative sum (e.g. financial return series). The check
  is now `sum != 0` via the helper.
* `wmape`'s docstring claimed `ValueError if actual_series contains
  some zeros`, but the implementation divides by `sum(|y_true|)` and
  only the all-zero case ever triggered the path. Docstring corrected.

The CHANGELOG entry for the parameter addition carries the breaking-
change marker per the convention discussed in unit8co#3080 (the post-mortem
on unit8co#3059), since the default behavior flips from raising to warning.

Adds a parametrized regression test covering all five metrics and an
explicit OPE-with-negative-sum test. Existing `test_ope_zero` and the
arre/marre legacy raise check are updated to opt into the legacy
behavior with `zero_division="raise"`.
@mahimn01 mahimn01 requested a review from dennisbader as a code owner May 26, 2026 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant