feat: Elo rating system (PlayerRatings elo() semantics) - #297
Closed
seonghobae wants to merge 5 commits into
Closed
Conversation
Implements the Kendall & Babington Smith (1940) circular-triad consistency test and coefficient of agreement u, as implemented by the eba R package 1.10-0 (circular.R / kendall.u.R, source READ; the 1940 paper and Alway's exact tables NOT READ, cited as origins per eba's manual pages). - Rust core scaling::circular_triads: T = C(n,3) - sum_j C(d_j,2) (integer arithmetic), T_max, T_exp = C(n,3)/4, zeta = 1 - T/T_max; EXACT null p-values for n <= 10 from embedded distributions (dyadic rationals, assert_eq!-pinned), continuity-corrected chi-square for n >= 11 (df = n(n-1)(n-2)/(n-4)^2). Documented divergences from eba: n = 2 and malformed/incomplete tournaments are rejected. - Rust core scaling::kendall_u: Sigma, u = 2*Sigma/(C(m,2)*C(n,2)) - 1, min_u, RAW chi-square (can be negative under continuity correction; only the p-value clamps), df = C(n,2)m(m-1)/(m-2)^2. Stricter than eba: every pair must have the same m >= 3 judges. - PyO3 bindings + Python wrappers circular_triads / kendall_u with CircularTriadsResult / KendallUResult dataclasses; input validation before casts. - 11 Rust tests (exact-Fraction oracle pins: 1940 dog example, n = 12 chi-square path vs scipy, table integrity sum = 2^C(n,2), negative raw chi2, error contracts, MC-500 invariants #[ignore]); 8 Python tests. Five mutants (drop pairing, T_max parity swap, drop opposite-tail, corr sign flip, drop Sigma correction) all EXECUTED and killed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Implements the Elo (1978) rating system as specified by the CRAN PlayerRatings 1.1-0 package's elo() (R/ratings.R + elo_c C kernel, both READ): batch-per-period updates where every expected score within a rating period uses the period-start ratings, per-game white advantage gamma, and PlayerRatings win/draw/loss and lag bookkeeping. Elo's 1978 book was NOT read and is cited as the origin per PlayerRatings. - Rust core mlsirm_core::scaling::elo_rating (EloResult with ratings, games, wins, draws, losses, lag); periods may be unsorted (grouped by ascending label, matching R split() ordering); self-play rejected and scalar K factor only (documented divergences). - PROVED: E_w + E_b = 1 identically for any finite gamma (the exponents are exact negations), so rating sums are conserved at n*init and an E_b = 1 - E_w refactor is a documented unkillable mutant. - Tests anchored to an executed exact-rational oracle: exact-fraction single/two-period fixtures (batch-semantics proof at kfac=400), float regression, closed-form nonzero-gamma pin, kfac=0, unsorted periods, fractional-score bookkeeping, saturation, error contract, and an MC-500 invariant suite (ignored by default). Five mutation kills executed: sequential-update, black-score flip, gamma sign flip, lag reset drop, logistic divisor. - PyO3 binding elo_rating; Python wrapper fast_mlsirm.elo_rating with (g, 4) [period, white, black, score] schedule, scalar gamma broadcast, and PlayerRatings defaults init=2200, kfac=27. 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 finding (High): the wrapper coerced the whole games array to float before the uint64 period cast, so distinct integer period labels above 2**53 silently merged into one rating period (wrong batching, wrong ratings), and out-of-u64 labels were accepted with only a NumPy warning. Fix: take period labels losslessly from integer-dtype input arrays, and reject float-path labels >= 2**53 (float(2**53+1) already rounds to 2**53, so that value is ambiguous). Regression test pins the crate's sequential-update ratings for labels 2**53 / 2**53+1 passed as uint64 and asserts ValueError on the float path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…integer bound Round-2 review finding (High): np.float32 games arrays lose integer fidelity above 2**24 (float16 above 2**11) before the float64 promotion, so distinct period labels could still silently merge under the previous 2**53-only guard. The float-path bound is now derived from the input dtype's mantissa (np.finfo(dtype).nmant). Regression test pins ValueError for float32 labels at 2**24 and exact crate ratings below the bound. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* feat(scaling): Glicko rating system with deviation inflation 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> * fix(scaling): float period-fidelity bound off by one mantissa bit 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> --------- 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.
Iteration 58 of the autonomous paper-implementation loop: Elo rating system (Elo, 1978), implemented from the READ CRAN PlayerRatings 1.1-0 sources (
R/ratings.Relo()+src/ratings.celo_c).Scope
mlsirm_core::scaling::elo_rating: batch-per-period Elo updates (all expectations within a period use period-start ratings), per-game white advantagegamma, PlayerRatings W/D/L bookkeeping (only scores exactly 1/0.5/0) and lag (periods since last appearance, reset for current-period players AFTER the played-before increment — first-timers end at 0).split()factor-level ordering) — mandated by spec-verify for fidelity.statuscarry-in,historyout of scope); index-based players (never-appearing players keep games=0, lag=0).fast_mlsirm.elo_rating(games, n_players, init=2200, kfac=27, gamma=None)with(g, 4)schedule and scalar-gamma broadcast; PlayerRatings defaults.Evidence discipline
R/ratings.R+src/ratings.cREAD in full (implementation source of record). Elo (1978) NOT READ — cited as origin per PlayerRatings documentation. Citation-governance header in scaling.rs.E_w + E_b = 1for any finite gamma (exponents are exact negations) — sosum(ratings) = n*initis conserved for ANY gamma, and anE_b = 1 - E_wrefactor is behaviorally unobservable (documented unkillable mutant; discriminating anchors target gamma sign/drop instead).Stacked on #296 (Kendall circular triads + u).
Adversarial impl-review outcome
Three review rounds were run against this PR:
gamesto float64 before casting periods touint64, so distinct integer period labels above 2^53 silently merged. Fixed in 912f8d8: integer-dtype 2-D inputs now take the period column losslessly viaraw[:, 0].astype(np.uint64); the float path rejects periods>= 2**53. Regressiontest_large_period_labels_exactpins two-period crate ratings for labels 2^53 / 2^53+1.2.0**np.finfo(raw.dtype).nmant). Regressiontest_float32_period_labels_rejected.2**nmantis over-strict by a factor of 2 (true exact-integer ceiling is2**(nmant+1)); this only rejects more inputs, never merges.TestElo: 7/7 pass.