Add metrics_rating prediction-quality metrics (PlayerRatings metrics()) - #304
Merged
Conversation
Rust core reimplementation of CRAN PlayerRatings 1.1-0 metrics() (R/ratings.R 936-957, READ; no journal paper exists -- CRAN source is the normative reference): per predictor column, binomial deviance on the cap-clamped predictions and RMSE/MAE on the RAW uncapped predictions (the R source quirk at lines 949/951), each times 100 and optionally divided by the 0.5-constant-predictor baseline. Numerator NaN removal is elementwise per pair; scaled baselines use the act-only row set (a different set when the predictor column has NaNs). The bdev baseline is implemented as the exact constant ln 2 (algebraic identity documented in the source, sub-ulp divergence from R's summation for non-0/1 act). Reduced scope (documented): which/sort/digits/drop presentation arguments are not implemented (full unrounded np x 3 matrix returned); na.rm=FALSE not implemented; Inf, empty per-column row sets, out-of- domain caps, and scale=TRUE with an all-0.5 act baseline are rejected where R would recycle or emit NaN/Inf. Tests: six exact-Fraction-oracle anchors (unscaled, cap quirk, scaled, NaN row sets, baseline-row-set killer, two-column stride pins with both columns pinned), error contract, MC-500 #[ignore] (scaled-vs- unscaled column-constant ratios with bdev ratio exactly ln 2; bitwise column-permutation invariance). Five mutation kills EXECUTED: cap-on-mse/mae, bdev-uncapped, stride transpose, missing sqrt, baseline pair-removal. Algebraic rearrangements (sqrt(a)/sqrt(b) vs sqrt(a/b)) are documented as unobservable. PyO3 binding metrics_rating (plain name) + Python wrapper with complex/object-dtype/shape validation before casts; TestMetrics; CHANGELOG. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Impl-review findings: numeric strings (plain, object-dtype, and cap tuples) were silently parsed as floats, bool arrays were accepted as 0/1, and 0-D scalars were promoted to 1-D by ascontiguousarray before the shape check, bypassing the documented contract. Validation now checks dtype kind and ndim before any cast; regression tests added. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Round-2 review finding: np.bool_ is not a bool subclass, so object arrays of np.bool_ bypassed the bool rejection and cast to 0/1 floats. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Round-3 adversarial review found that object-dtype arrays containing None were silently cast to NaN by astype(float64), bypassing the explicit-missing contract (missing values must be passed as np.nan). None (and any element that is str/bytes/bool/np.bool_) is now rejected with ValueError before the cast, with regression tests for both act and pred paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Round-4 adversarial review found that np.ma.MaskedArray inputs lost their mask through np.asarray, so masked missing values were silently counted as observed. Masked arrays are now rejected with ValueError before conversion; missing values must be encoded as explicit np.nan. Regression tests cover both act and pred paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
3 tasks
Contributor
Author
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
1 participant
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.
Implements CRAN PlayerRatings 1.1-0
metrics()(R/ratings.R lines 936-957, READ; no journal paper exists — the CRAN source is the normative reference) asmetrics_rating: per-predictor-column binomial deviance / RMSE / MAE for binary-outcome forecasts, times 100, optionally scaled by the 0.5-constant-predictor baseline.Verified R semantics preserved
pred[,i], notpredc) — the key quirk, pinned by an anchor whose mutants were EXECUTED-killed.Reduced scope (documented in the source):
which/sort/digits/droppresentation args skipped (full unrounded np x 3 matrix returned);na.rm=FALSEskipped; Inf / empty row sets / out-of-domain caps /scale=TRUEwith all-0.5 act are rejected where R recycles or yields NaN/Inf.Testing
#[ignore](scaled/unscaled column-constant ratios, bdev ratio exactly ln 2; bitwise column-permutation invariance).Stacked on #303 (elom).
Adversarial impl-review outcome
Five review rounds (independent adversarial reviewer agents), all findings fixed:
np.bool_bypassed the bool rejection (np.bool_is not aboolsubclass).Nonesilently cast to NaN; now rejected (missing must be explicitnp.nan).np.ma.MaskedArraymasks silently dropped bynp.asarray; masked arrays now rejected.Rust core reviewed CLEAN in Round 1 (no core findings in any round). All regression tests included in
TestMetrics.