Skip to content

fix(r_squared): implement the documented zero_division parameter - #804

Open
hassaanch23 wants to merge 1 commit into
huggingface:mainfrom
hassaanch23:fix/r-squared-zero-division
Open

fix(r_squared): implement the documented zero_division parameter#804
hassaanch23 wants to merge 1 commit into
huggingface:mainfrom
hassaanch23:fix/r-squared-zero-division

Conversation

@hassaanch23

Copy link
Copy Markdown

Closes #803.

The problem

r_squared documents a zero_division parameter that was never implemented:

zero_division: Which value to substitute as a metric value when encountering zero division.
    Should be one of 0, 1, "warn". "warn" acts as 0, but the warning is raised.

zero_division appeared exactly once in the file — in that docstring. _compute didn't accept it, so passing it raised TypeError, and the case it exists to control was unguarded:

sst = np.sum((references - mean_references) ** 2)
r_squared = 1 - (ssr / sst)

When every reference is identical there is no variance to explain, sst is zero, and R^2 is undefined. numpy returns -inf — or nan when predictions match exactly — with only a RuntimeWarning, and round() passes it through.

predictions references before after (default) sklearn.r2_score
[1.1, 2.1, 2.9, 4.2] [1, 2, 3, 4] 0.986 0.986 0.986
[1, 2, 3, 4] [5, 5, 5, 5] -inf 0.0 + warning 0.0
[5, 5, 5, 5] [5, 5, 5, 5] nan 0.0 + warning 1.0

Both values propagate: one constant-reference batch turns an averaged score into nan for a whole run, and -inf dominates any mean. Nothing raises, so it reads as a modelling outcome rather than an undefined computation.

The change

Implements zero_division as documented — 0, 1, or "warn" — and raises ValueError on anything else rather than ignoring it.

The default is "warn": the previously silent case becomes visible instead of quietly becoming a number the caller didn't choose. Callers who know which convention they want can pass 0 or 1 and silence it. Note the two degenerate cases differ under sklearn (0.0 when predictions miss, 1.0 when they match exactly), which is exactly why this is a caller choice rather than a fixed substitution.

Non-degenerate inputs are untouched and still agree with sklearn.metrics.r2_score to 3 dp.

Tests

Adds doctest examples for both settings, which is how metrics here are covered. tests/test_metric_common.py -k r_squared passes (1 passed, 1 skipped).

One note for anyone reproducing: the test suite doesn't run on Python 3.12+ — tests/utils.py imports distutils, removed in 3.12. Verified on 3.11.

Not changed

_DESCRIPTION says "The R^2 value ranges from 0 to 1", which isn't true for a fit worse than the mean — sklearn returns negative values there, as does this implementation. Left alone to keep this PR to one thing; happy to fix it here or separately.

`_KWARGS_DESCRIPTION` documents a `zero_division` argument:

    zero_division: Which value to substitute as a metric value when
        encountering zero division. Should be one of 0, 1, "warn". "warn"
        acts as 0, but the warning is raised.

but `_compute(self, predictions=None, references=None)` never accepted it, so
passing it raised TypeError, and the case it exists to control was unhandled.

When every reference is identical there is no variance to explain, so the sum
of squared total is zero and R^2 is undefined. Left to numpy the division
returned -inf, or nan when the predictions matched exactly, with only a
RuntimeWarning. Both propagate silently through any downstream aggregation:
one constant-reference batch turns an averaged score into nan.

    predictions=[1,2,3,4], references=[5,5,5,5]   before -inf   sklearn 0.0
    predictions=[5,5,5,5], references=[5,5,5,5]   before  nan   sklearn 1.0

`zero_division` now works as documented, defaulting to "warn" so the
previously silent case becomes visible rather than changing quietly to a
number the caller did not choose. An invalid value raises ValueError instead
of being ignored.

Non-degenerate inputs are untouched and still agree with
sklearn.metrics.r2_score.

Adds doctest examples for both settings, which is how metrics in this repo are
covered; tests/test_metric_common.py passes for r_squared.
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.

r_squared: documented zero_division parameter is not implemented; constant references return -inf/nan

1 participant