Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate rating system from Topcoder to Elo-MMR #1751

Merged
merged 24 commits into from
Aug 17, 2021
Merged

Migrate rating system from Topcoder to Elo-MMR #1751

merged 24 commits into from
Aug 17, 2021

Conversation

int-y1
Copy link
Contributor

@int-y1 int-y1 commented Aug 14, 2021

Elo-MMR is a rating system that has more desirable properties than Topcoder's rating system. For example, Elo-MMR converges faster, prevents users from farming volatility, and has a more reasonable performance calculation.

Change in the rating curve (ratings rounded to the nearest 100x+50): ratingcurve

Change in each user's rating (i.e. users can expect their rating to increase by +100):
ratingdelta

Due to the expected +100 rating increase, I modified the rating cutoffs:

  • Old: RATING_VALUES = [1000, 1200, 1500, 1800, 2200, 3000]
  • New: RATING_VALUES = [1000, 1300, 1600, 1900, 2400, 3000]

Change in performance (on my virtual machine):

  • It takes 8 seconds to rate all 111 contests using Topcoder.
  • It takes 12 seconds to rate all 111 contests using Elo-MMR. The biggest bottlenecks are recalculate_ratings() (4.5 seconds), and the query for historical_p (1.5 seconds).

@codecov-commenter
Copy link

codecov-commenter commented Aug 14, 2021

Codecov Report

Merging #1751 (0517a63) into master (64da46b) will decrease coverage by 0.29%.
The diff coverage is 18.51%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1751      +/-   ##
==========================================
- Coverage   46.52%   46.22%   -0.30%     
==========================================
  Files         221      222       +1     
  Lines       12461    12612     +151     
==========================================
+ Hits         5797     5830      +33     
- Misses       6664     6782     +118     
Impacted Files Coverage Δ
judge/models/tests/test_profile.py 100.00% <ø> (ø)
judge/views/api/api_v1.py 23.52% <0.00%> (ø)
judge/views/api/api_v2.py 47.79% <0.00%> (ø)
judge/ratings.py 21.19% <17.39%> (+3.01%) ⬆️
judge/migrations/0123_contest_rating_elo_mmr.py 18.33% <18.33%> (ø)
judge/models/contest.py 95.48% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64da46b...0517a63. Read the comment docs.

Copy link
Member

@quantum5 quantum5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise looks pretty good.

judge/ratings.py Outdated Show resolved Hide resolved
judge/ratings.py Outdated Show resolved Hide resolved
judge/ratings.py Outdated Show resolved Hide resolved
judge/ratings.py Outdated Show resolved Hide resolved
judge/ratings.py Outdated Show resolved Hide resolved
judge/ratings.py Outdated Show resolved Hide resolved
Copy link
Member

@Ninjaclasher Ninjaclasher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also create a PR to https://github.com/DMOJ/docs/blob/master/docs/site/api.md with any API changes.

Copy link
Member

@quantum5 quantum5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks fine to me. Has anyone tested the migration? If it works I am going to approve.

judge/ratings.py Outdated Show resolved Hide resolved
judge/ratings.py Outdated Show resolved Hide resolved
Copy link
Member

@Xyene Xyene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and applied to prod.

Thanks everyone who worked on this!

@Xyene Xyene merged commit 440fca6 into DMOJ:master Aug 17, 2021
@int-y1 int-y1 deleted the elo-mmr branch August 17, 2021 02:12
jtyliu added a commit to jtyliu/online-judge that referenced this pull request Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants