Fix operator precedence bug in lambda_ij calculation #60
Conversation
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
In resolve_model(), line 263 used 'n / pp*2' which evaluates as '(n/pp)*2' due to Python's left-to-right evaluation for operators with same precedence. The formula comment on line 256 clearly states: lambda_ij = n_ij / (p_i + p_j)^2, i != j And line 262 correctly uses 'pp**2' for the same formula in lbda_ii. This change fixes lbda_ij to use 'pp**2' (exponentiation) instead of 'pp*2' (multiplication), matching the mathematical formula specification.
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.
Summary
BradleyTerryMlePairedCompSubjectiveModel.resolve_model()wheren / pp*2was evaluated as(n/pp)*2instead of the intendedn / pp**2Problem
In
sureal/pc_subjective_model.pyline 263, the code reads:Due to Python's operator precedence (division and multiplication have equal precedence and are evaluated left-to-right), this computes
(n / pp) * 2, which is incorrect.The mathematical formula specified in the comment on line 256 states:
Line 262 correctly implements this for
lbda_ii:Fix
Changed line 263 from:
to:
Impact
This affects the covariance matrix calculation (
cova_p,cova_v) and the standard deviation (stdv_p,stdv_v) outputs of the Bradley-Terry MLE model. The quality scores themselves are not affected, only their uncertainty estimates.Note: The existing tests will need their expected values updated since they were calibrated against the buggy implementation.