feat(scaling): Glicko-2 rating system (Glickman 2022 + PlayerRatings glicko2) - #301
Closed
seonghobae wants to merge 5 commits into
Closed
feat(scaling): Glicko-2 rating system (Glickman 2022 + PlayerRatings glicko2)#301seonghobae wants to merge 5 commits into
seonghobae wants to merge 5 commits into
Conversation
Implements the Glicko rating system as a Rust core (mlsirm_core::scaling::glicko_rating) with a thin PyO3 binding and Python wrapper. Sources READ: Glickman's 'The Glicko system' technical note (worked example reproduced to full float64 precision) and CRAN PlayerRatings 1.1-0 glicko()/glicko_c. Glickman (1999), the derivation paper, was NOT read and is cited as the origin per both READ sources. - Batch-per-period Step 2 updates with opponent-g weighting and the new-variance rating step; participant-only Step 1b inflation RD = min(sqrt(RD^2 + (lag+1) c^2), rdmax). - Per-player init_rating/init_dev arrays (heterogeneous RDs); results cover ALL 0..n players (documented no-status divergence from R). - Documented non-identity: no rating-sum conservation (pinned by test). - Tests anchored to an executed float64 oracle: Glickman worked-example anchor, two-period inflation/lag/idle-player full-vector pins, rdmax clamp, gamma exact pins, unsorted periods, fractional score, error contract, MC-500 (#[ignore]). - Seven executed mutation kills: opponent-g swap, inflation off-by-one, clamp drop, stale-variance update, missing q^2, gamma sign, all-player inflation. - Python wrapper inherits the Elo period-label fidelity contract (integer-dtype lossless u64 path; dtype-derived float bound). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
np.finfo(dtype).nmant excludes the implicit leading bit, so the exact-integer ceiling of a float dtype is 2**(nmant + 1), not 2**nmant. The elo/glicko wrappers were rejecting exactly representable period labels one power of two early (float32 at 2**23, float64 at 2**52). Bound is now 2**(nmant + 1) with the >= comparison kept (2**53 itself is ambiguous because 2**53 + 1 rounds onto it). Boundary tests pin acceptance of 2**24 - 1 (float32) and 2**53 - 1 (float64) via crate game tallies, killing a 2**nmant mutant. Found by adversarial implementation review of PR #298. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…glicko2) Rust core glicko2_rating with batch-per-period updates on the Glicko-2 scale: participant-only pre-period variance inflation phi^2 <- min(phi^2 + lag * sigma^2, (q rdmax)^2) (Glicko-2 lag, not Glicko-1 lag+1; R source comment pinned), per-player volatility via Glickman's Step-5 Illinois iteration (eps 1e-6, endpoint A; DERIVED: f(x) = -1/2 d/dx of PlayerRatings' penalized nllh, so the Illinois root matches R's optimum), tau == 0 volatility freeze, volatility ceiling q * rdmax, per-game white advantage gamma, W/D/L and lag bookkeeping. Documented R-vs-note deviation: idle players get no per-period Step-6 growth; lag * sigma^2 applies at next participation. Anchored to an executed float64 oracle: Glickman worked-example anchor (r'=1464.05, RD'=151.52, sigma'=0.059996) with heterogeneous init, two-period inflation/lag/idle pins, rdmax + volatility-ceiling clamps, gamma, unsorted-period, fractional-score + tau-0, return-after-idle. Nine executed mutation kills (own-g swap, lag off-by-one, variance-clamp drop, volatility-clamp drop, skipped volatility update, stale-sigma inflation, Illinois endpoint swap, gamma sign, rating-before-deviation). PyO3 binding glicko2_rating; Python wrapper fast_mlsirm.glicko2_rating returning Glicko2Result, inheriting the Elo/Glicko period-label fidelity contract. cargo 794 pass; pytest 305 pass. 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 |
- Python wrapper: wrap int(n_players), gamma asarray, and float(tau)/ float(rdmax) in narrow try/except -> ValueError so callers see the documented exception type instead of leaked TypeError/OverflowError (red-green mutation kill EXECUTED: reverting the n_players guard fails the error-contract test). - PyO3 binding: usize::try_from for white/black player ids instead of 'as usize' truncation on 32-bit targets; PyValueError on overflow. - Error-contract coverage: Rust g2_error_contract adds non-finite init arrays, non-finite/negative rdmax, negative score, and at/above the ln(10)/400*rdmax volatility ceiling boundary; Python TestGlicko2 adds 18 cases (out-of-range/negative index, score bounds, gamma shape/ non-finite/complex/object, non-finite tau/rdmax, inf/None n_players, None tau/rdmax, init NaN/length mismatch, 10000-player cap). cargo g2_: 11 pass; pytest TestGlicko2: 5 pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* feat(scaling): Stephenson rating system (PlayerRatings steph()) 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> * fix(stephenson): impl-review round-1 fixes (1 High, 2 Medium) - 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> --------- 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.
Glicko-2 rating system (iteration 60)
Implements
glicko2_rating— Glicko-2 with per-player rating volatility — as a Rust core + thin PyO3/Python wrapper.Sources (citation governance)
glicko2()(R/ratings.Rlines ~412-586) andglicko2_cC kernel — batch-period semantics, participant-onlylag * sigma^2inflation (source comment: "nlag*(cvols^2) in Glicko-2, (nlag+1)*(cval^2) in Glicko"),q * rdmaxvolatility ceiling, tau>0 gate, gamma signs, tallies.f(x)equals-1/2the derivative of PlayerRatings' penalized negative log-likelihood, so the Illinois root coincides with R'soptimize()optimum.phi' = sqrt(phi^2 + sigma^2)) to idle players every period; PlayerRatings — and this port — defer idle growth vialag * sigma^2at next participation.Verification
n_playersdropped from the Rust signature).lag+1inflation, variance-clamp drop, volatility-clamp drop, skipped volatility update, stale-sigma Step-6 inflation, Illinois endpoint-B return, gamma sign flip, rating-before-deviation order.#[ignore], executed: pass).Stacked on #298 (
seonghobae-glicko).Adversarial impl-review outcome
TypeError/OverflowError instead of the documented ValueError for
int(n_players)with inf, object-dtype gamma, andfloat(tau/rdmax)with None. (2) Error-contract tests claimed exhaustive coverage but
omitted non-finite init/rdmax, negative score, and 18 Python-side cases.
(3) PyO3 binding truncated u64 player ids with
as usizeon 32-bittargets. Core algorithm, Illinois volatility loop, ordering, lag
semantics, and all paper anchors confirmed clean.
wrapper (red-green mutation kill EXECUTED);
usize::try_fromwithPyValueError in the binding; Rust g2_error_contract + Python
TestGlicko2 extended incl. the ln(10)/400*rdmax volatility-ceiling
boundary (at-ceiling Ok, just-above Err).