feat(scaling): Glicko rating system with deviation inflation - #298
Merged
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>
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 |
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>
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
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.
Iteration 59 of the autonomous paper-implementation loop. Stacked on #297 (Elo).
Summary
Implements the Glicko rating system (Glickman) as a Rust core
mlsirm_core::scaling::glicko_ratingwith a thin PyO3 binding and Python wrapperfast_mlsirm.glicko_rating.Citation governance
gk_paper_anchor_ga.R/ratings.Rglicko()+src/ratings.cglicko_c— batch-per-period reference (cval inflation, rdmax clamp, gamma, W/D/L + lag bookkeeping).Model
Per period (ascending label, batch semantics): participant-only variance inflation
v = min(v + (lag+1)c², rdmax²);g = 1/sqrt(1 + 3(q/π)²v)post-inflation; per-game accumulation with opponent g; variance update first, then rating with the new variance. Documented non-identity: no rating-sum conservation (asymmetric opponent-g weighting; pinned ≠ 6600 by test).Verification
gk_mc_500(#[ignore]) executed once, pass.Adversarial implementation review (2 rounds)
Round 1 ? FINDINGS (
files/glicko_impl_review.md): confirmed all core semantics clean vs PlayerRatings R/C reference (participant-only pre-period inflation with rdmax clamp, post-inflation g for all players, opponent-g accumulation, variance-before-rating update order, gamma signs, exact-only tallies, lag order; GB deviation-vector pin distinguishes MU9, GD pins kill gamma sign swap). One Medium defect: the float period-fidelity bound used2.0 ** np.finfo(dtype).nmant?nmantexcludes the implicit leading bit, so exactly representable labels were rejected one power of two early (float32 at 2^23, float64 at 2^52) in both the elo and glicko wrappers.Fix (bb2a441): bound is now
2.0 ** (np.finfo(dtype).nmant + 1)(float64 2^53, float32 2^24, float16 2^11) with the>=comparison kept (2^53 itself stays rejected: 2^53 + 1 aliases onto it). Boundary tests added for both elo and glicko pin acceptance of 2^24 - 1 (float32) and 2^53 - 1 (float64) via crate game tallies; these asserts FAIL under the old2**nmantcode, killing that mutant.Round 2 ? CLEAN (
files/glicko_impl_review_round2.md): fix verified correct for all three float dtypes,>=semantics confirmed, boundary tests confirmed mutation-killing, TestGlicko + TestElo 11/11 pass, no new defects in the fix diff.