fix(r_squared): implement the documented zero_division parameter - #804
Open
hassaanch23 wants to merge 1 commit into
Open
fix(r_squared): implement the documented zero_division parameter#804hassaanch23 wants to merge 1 commit into
hassaanch23 wants to merge 1 commit into
Conversation
`_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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #803.
The problem
r_squareddocuments azero_divisionparameter that was never implemented:zero_divisionappeared exactly once in the file — in that docstring._computedidn't accept it, so passing it raisedTypeError, and the case it exists to control was unguarded:When every reference is identical there is no variance to explain,
sstis zero, and R^2 is undefined. numpy returns-inf— ornanwhen predictions match exactly — with only aRuntimeWarning, andround()passes it through.sklearn.r2_score[1.1, 2.1, 2.9, 4.2][1, 2, 3, 4][1, 2, 3, 4][5, 5, 5, 5][5, 5, 5, 5][5, 5, 5, 5]Both values propagate: one constant-reference batch turns an averaged score into
nanfor a whole run, and-infdominates any mean. Nothing raises, so it reads as a modelling outcome rather than an undefined computation.The change
Implements
zero_divisionas documented —0,1, or"warn"— and raisesValueErroron 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 pass0or1and 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_scoreto 3 dp.Tests
Adds doctest examples for both settings, which is how metrics here are covered.
tests/test_metric_common.py -k r_squaredpasses (1 passed, 1 skipped).One note for anyone reproducing: the test suite doesn't run on Python 3.12+ —
tests/utils.pyimportsdistutils, removed in 3.12. Verified on 3.11.Not changed
_DESCRIPTIONsays "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.