feat(scaling): multiplayer Elo rating (PlayerRatings elom) - #303
Closed
seonghobae wants to merge 5 commits into
Closed
feat(scaling): multiplayer Elo rating (PlayerRatings elom)#303seonghobae wants to merge 5 commits into
seonghobae wants to merge 5 commits into
Conversation
Rust core stephenson_rating extending Glicko with per-game neighborhood variance (ngames*hval^2), per-game bonus bval/100 on both sides, participants-only lambda drift toward opponents, and (lag+1)*cval^2 per-period deviation-variance inflation clamped at rdmax^2. Normative source: CRAN PlayerRatings 1.1-0 R driver (ratings.R 591-737) + C kernel (ratings.c stephenson_c 157-202), both READ and line-cited; no journal paper exists (Kaggle-2010 provenance noted as NOT independently verifiable). PyO3 binding + NumPy wrapper with PlayerRatings defaults. Tests anchored to an EXECUTED faithful oracle port (S1 heterogeneous init, S2 two-period draw/lag, S3 full knobs, S4 rdmax clamp + prior-run continuation, S5 bval symmetry, lambda=0 contrast; 1e-12 pins), 500-rep MC invariants (#[ignore]), and five EXECUTED mutation kills: bval drop, lambda sign flip, per-game hval scaling drop, (lag+1)->lag, opponent-g->own-g. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Enforce the 2..=10000 n_players cap in the Python wrapper BEFORE any length-n allocation (High: huge n_players previously attempted the allocation instead of raising ValueError). - Preserve integer fidelity for white/black player-id columns: integer dtypes cast directly to u64; float/object inputs are rejected at or above the dtype's exact-integer bound before the uint64 cast, matching the existing period-label contract (Medium). - Guard u64 counter overflow in the Rust core: init_games/init_lag values that could overflow across the run's increments now return Err instead of panicking (debug) or wrapping (release) (Medium). Regression tests: Rust st_error_contract overflow cases; Python fidelity + pre-allocation cap cases. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Implements elom_rating: nn-seat event ratings with rank base scores, per-period single update K*(actual - expected) with expected summing (r_p - event mean rating)/40, constant-K or kriichi experience-decay K factor, placing mode, and games/places/lag bookkeeping. Normative sources (READ): CRAN PlayerRatings 1.1-0 R/ratings.R lines 739-932 (elom driver), src/ratings.c lines 45-80 (elom_c kernel), R/ratings.R lines 1006-1020 (kriichi). No journal paper exists for this system. Faithfully reproduces the R quirk that partial events shrink the ORIGINAL base exactly once regardless of empty-seat count (sbase <- basev resets inside the shrink loop, R:855-866). REDUCED-SCOPE vs R (documented in the core header): player == -1 iff score is NaN (jointly enforced), sorted periods required, in-event duplicate players rejected, kriichi bounds gv > 0 and 0 < kv <= 1. Tests: exact-value anchors E1-E9 (dyadic rationals, hand-derived oracle executed), error contract, MC-500 permutation invariance (#[ignore]); 6 mutation kills executed (K-scaling, event-mean, cumulative-shrink, kriichi games-timing, tie-rank, per-event-update). PyO3 binding elom_rating + numpy-validating Python wrapper. 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 |
An unsigned id above i64::MAX wrapped to a negative value through the int64 narrowing cast (uint64::MAX -> -1) and was silently treated as the empty-seat sentinel instead of being rejected. Impl-review finding 1 (Medium). Adds a regression test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…)) (#304) * Add metrics_rating prediction-quality metrics (PlayerRatings metrics()) 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> * Reject string/bool and 0-D inputs in metrics_rating wrapper 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> * Reject object-dtype np.bool_ elements in metrics_rating wrapper 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> * fix: reject object-dtype None in metrics_rating inputs 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> * fix: reject masked arrays in metrics_rating inputs 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> --------- 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.
Multiplayer Elo rating (PlayerRatings
elom())Iteration 62 of the paper-implementation loop. Stacked on #302 (base
seonghobae-stephenson).What
elom_rating: nn-seat multiplayer Elo events with rank base scores. Per period each player receives ONE updateK*(ascore - escore)whereascoresums the seat base score andescoresums(r_p - event mean rating)/40over the player's events. K is a constant or the kriichi experience decaymax(kv, 1 - (1-kv)*games/gv)using PRE-period cumulative games.placing=Trueranks by lowest score. Bookkeeping:games,places(n x nn finish counts),lag.Sources (citation governance)
R/ratings.Rlines 739-932 (elomdriver) — READ, normativesrc/ratings.clines 45-80 (elom_ckernel) — READ, normativeR/ratings.Rlines 1006-1020 (kriichi) — READ, normativeFaithfully reproduces the R quirk that partial events shrink the ORIGINAL base exactly once regardless of empty-seat count (
sbase <- basevresets inside the shrink loop, R:855-866) — my first oracle draft applied the shrink cumulatively and was corrected against the verbatim R source.Spec-verify (adversarial, BEFORE implementation)
Verdict REDUCED-SCOPE with 12 findings, all adopted (spec Rev-2), including the BLOCKER on missing-seat semantics (adopted contract:
player == -1iff score NaN, jointly enforced), nn=2 acceptance, MC tolerance semantics, and a new anchor E9 forcing an executed kill for the per-event-update mutant.Tests (every assert reads crate outputs)
#[ignore](EXECUTED, passes)Suites
cargo test -p mlsirm-core --lib: 812 passed (10 new)pytest tests/test_paper_features.py: 313 passed (3 new)Adversarial impl-review outcome
Round 1: FINDINGS ? 1 Medium: the Python wrapper cast unsigned player arrays directly to int64, so
np.uint64.maxwrapped to-1and was silently treated as the empty-seat sentinel. Fixed in 0a47ead (guard rejecting unsigned ids abovei64::MAX+ regression test). The reviewer independently recomputed anchors E2/E8/E9 from the R/C source and matched all pins.Round 2: CLEAN ? fix verified (reproduction now raises ValueError; uint32/valid-uint64/int64-sentinel probes pass; no new findings).