Add Rank Centrality paired-comparison estimator - #293
Closed
seonghobae wants to merge 5 commits into
Closed
Conversation
Implements choix 0.4.1's lsr.py dense pairwise path (source READ;
Maystre & Grossglauser 2015 NOT READ, cited as described by choix):
one-shot spectral estimate and iterative MLE from an n x n win-count
matrix. Rust core (mlsirm_core::scaling::{lsr_pairwise, ilsr_pairwise})
with Gaussian-elimination statdist guarded by positivity, sum, and
residual checks; overflow from huge counts/alpha raises instead of
returning NaN. I-LSR at alpha=0 reproduces the Bradley-Terry MLE
(cross-algorithm anchor vs bradley_terry_mm); alpha>0 regularization
semantics deliberately differ (chain-rate vs Dirichlet-MAP, per source).
Pins from an EXECUTED exact-Fraction/mpmath oracle cross-checked with
pip choix 0.4.1 (<= 2.2e-13). Six mutation kills EXECUTED (chain
transpose, dropped diagonal subtraction, dropped centering,
sum-n normalization via weights pins, denominator collapse via I-LSR
pins with the one-shot-unobservable limitation documented, tol*n vs tol
via a separating iteration-count fixture). MC-500 recovery test
(#[ignore]) passing. cargo 745 pass; pytest 273 pass.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… rejection The relative pivot threshold in the stationary-distribution solve compared an O(1) sum-constraint pivot against an O(max count) global scale, falsely rejecting validly connected win matrices with globally huge counts (impl-review finding). Normalize the generator to unit max magnitude after the overflow guard; the stationary distribution is invariant under global rescaling of transition rates. Regression asserts: base vs base*1e20 params/weights equal to 1e-12; asymmetric 1e150 ILSR finite. The overflow fixture is now a genuinely overflowing n=4 matrix (row sums -> inf); the previous n=3 all-1e308 fixture never overflowed and is correctly accepted post-fix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…atrix Mirror of the Rust-side fixture change in 343fc2f: the n=3 all-1e308 matrix never overflowed and is correctly accepted after the scale-invariance fix; use n=4 at 1.7e308 (row sums -> inf) and add a Python-side scale-invariance regression assert. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Port choix 0.4.1's rank_centrality (Negahban, Oh, & Shah, 2017, as implemented by choix's continuous-time win-ratio chain; choix source READ, paper's discrete-time walk NOT implemented) to the Rust core: - scaling::rank_centrality builds the Markov chain with regularized win-ratio rates (alpha + w_ij) / (2*alpha + w_ij + w_ji) from a pre-transform counts snapshot, with explicit Err on disconnected graphs at alpha = 0 and on overflowing counts or ratio denominators (choix silently degrades to near-zero ratios there). - Extract the shared stationary-distribution solver statdist_params (normalization, partial-pivot Gaussian elimination, guards, centered log transform) from the LSR pass and reuse it, so LSR/I-LSR and Rank Centrality share one verified solver. - PyO3 binding rank_centrality; Python wrapper returning LsrResult. - Tests pin EXECUTED exact-Fraction oracle anchors (3x3 and 4x4 at alpha 0 and 1/2, one-sided, disconnected-with-alpha), exact scale invariance at alpha = 0 only, error contract, and an ignored 500-rep Monte-Carlo recovery check (measured worst MAE 0.0912, bound 0.15); cross-checked against pip choix 0.4.1 to 2e-16. Six mutation kills executed (transposed ratio, missing ratio transform, half-updated denominator, denominator-c-only, dropped normalization, dropped centering). 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 |
…lsr_rankings) (#294) * Add Plackett-Luce rankings LSR/I-LSR estimators (choix lsr_rankings/ilsr_rankings) Rust cores scaling::lsr_rankings (one-shot) and scaling::ilsr_rankings (iterative MLE) for full/partial rankings in CSR layout, porting choix 0.4.1 exactly (source READ; Maystre & Grossglauser 2015 NOT READ, cited as-cited per choix docstrings). Python wrappers accept lists of rankings, validate before unsigned casts (negatives, non-integers, length<2 rejected), and return LsrResult. Documented divergences from choix: length<2 rankings rejected (choix no-ops), within-ranking duplicates rejected (choix accepts if connected), negative indices rejected (Python would wrap). Tests: exact rational anchors from an executed exact-Fraction/mpmath oracle (full + partial fixtures; the partial fixture is the only one that can see a wrong all-items denominator), bit-exact length-2 equivalence with lsr_pairwise, I-LSR fixed-point pins (atol 1e-7 = oracle-measured margin) + iteration-count pins (8/11 at tol=1e-8) + weights==exp_transform(params) invariant, full error contract incl. disconnected graph and alpha overflow, MC-500 recovery (#[ignore], bound 0.2 vs measured worst 0.1440). Five mutation kills executed (MU1 stale denominator, MU2 transpose, MU3 full-ranking losers, MU4 all-items sum, MU5 uniform I-LSR worths). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Fix PL rankings validation gaps from adversarial review - Cap n at 10000 in rankings_validate: the dense O(n^2) chain would otherwise attempt terabyte allocations and abort the process on tiny inputs like lsr_rankings([[0,1]], 1_000_000) (finding 1, High). - Reject np.bool_ items alongside Python bool (finding 2, Medium). - Catch OverflowError from int(x) so infinite items raise ValueError per the wrapper contract (finding 3, Low). Regression tests added on both the Rust error contract and the Python validation test. 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 54 of the autonomous paper-implementation loop. Stacked on #292 (base
seonghobae-lsr).Rank Centrality (Negahban, Oh, & Shah, 2017 — choix 0.4.1 port)
Adds
scaling::rank_centralityto the Rust core with a thin PyO3 binding and Python wrapper (fast_mlsirm.rank_centrality->LsrResult).Source basis (citation governance): the choix 0.4.1
rank_centralitysource was READ and is the implemented contract. The Negahban–Oh–Shah paper's discrete-time max-degree random walk is NOT what choix computes; only the choix continuous-time win-ratio chain is implemented, and this divergence is documented in the code header.Algorithm: from an n x n win-count matrix, take a pre-transform counts snapshot
alpha + wins, then set each loser-to-winner rate to the regularized win ratio(alpha + w_ij) / (2*alpha + w_ij + w_ji), diagonal = negative off-diagonal row sum, and solve for the stationary distribution. The stationary solver (unit-max normalization, partial-pivot Gaussian elimination, positivity/sum/residual guards, centered log transform) is extracted from the LSR pass into a sharedstatdist_paramshelper so LSR/I-LSR/Rank-Centrality use one verified solver (spec-verify mandate).Documented divergences from choix: explicit
ValueErroron overflowing counts or ratio denominators (alpha = 1e308overflows the denominator2*alpha; choix silently produces near-zero ratios), and all-zero wins matrices are rejected even atalpha > 0.Exact scale invariance holds at
alpha = 0only (ratios are homogeneous of degree 0 in the counts); pinned by test.Verification
mlsirm-core --lib749 pass; pytesttests/test_paper_features.py275 pass. MC-500#[ignore]recovery test: measured worst MAE 0.0912, bound 0.15.Adversarial implementation review (post-merge-to-branch)
Independent adversarial review of commit 6fff3c4 — VERDICT: CLEAN (round 1, no findings). Reviewer re-derived the choix ratio-chain algorithm from source (including the diagonal ratio-then-subtract cancellation), diffed the extracted
statdist_paramshelper against the pre-refactor LSR tail (byte-identical guards/thresholds), confirmed all 5 spec-verify mandatory changes, verified every test assert reads crate outputs against the executed exact-Fraction oracle pins, probed overflow paths (no NaN/Inf escape found within the validated input contract), and re-ran the scaling suite (20 pass) plus the ignored MC-500 recovery test (pass).