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

glicko_rating_period is probably supposed to imply decay_deviation #3

Closed
asyncth opened this issue Dec 17, 2022 · 7 comments · Fixed by #4
Closed

glicko_rating_period is probably supposed to imply decay_deviation #3

asyncth opened this issue Dec 17, 2022 · 7 comments · Fixed by #4
Labels
bug Something isn't working

Comments

@asyncth
Copy link
Contributor

asyncth commented Dec 17, 2022

As far as I understand it, Glicko paper requires to increase RD at the beginning of every rating period in Step 1 before changing it again in Step 2, regardless of whether the player played games in previous rating period or not, since it is said that Step 1 should be done for each player and Step 2 specifies RD as a rating deviation from Step 1:

Step 1. Determine the rating and RD for each player at the start of the new rating period based on their rating and RD at the end of the previous period.

[In Step 2] Assume that the player’s pre-period rating is r, and the ratings deviation is RD determined from Step 1.

Which I believe means that glicko_rating_period needs to run step 1 as well, the same equation that decay_deviation uses. This would be similar to Glicko-2 where Step 6 is ran in both glicko2 and glicko2_rating_period.

@atomflunder atomflunder added the bug Something isn't working label Dec 17, 2022
@atomflunder
Copy link
Owner

Hi, thanks for the bug report!

I think you are right, this seems to be an oversight. If you want to submit a PR to fix this yourself, you are welcome to. Otherwise I will do so sometime later this weekend.

I think it would make sense to run the same calculation in the normal glicko function, too. Since it kind of is a rating period with one match, in a sense.

@asyncth
Copy link
Contributor Author

asyncth commented Dec 17, 2022

Trying to fix this now. I wanted to put step 1 calculation into new_deviation functions, but calculating the new rating in step 2 seems to require deviation that was calculated in step 1, but not yet changed by step 2, which means that I probably can't just put the calculation into new_deviation and re-order calculations of new deviation and new rating in glicko function. Should I simply put it into glicko and perhaps change the name of the deviation argument of new_deviation and new_rating functions to something like pre_deviation?

@atomflunder
Copy link
Owner

Should I simply put it into glicko and perhaps change the name of the deviation argument ofnew_deviation and new_rating functions to something like pre_deviation?

Yes, I think that would make the most sense. The calculated RD from step 1 also would be needed in the g_value function if I understand it correctly. Thanks for fixing it!

@asyncth
Copy link
Contributor Author

asyncth commented Dec 17, 2022

Also, I assume this will break the test_glicko_rating_period test, the paper says it's meant to demonstrate step 2 and it doesn't follow step 1. What do I do with it?

@atomflunder
Copy link
Owner

atomflunder commented Dec 17, 2022

Also, I assume this will break the test_glicko_rating_period test, the paper says it's meant to demonstrate step 2 and it doesn't follow step 1. What do I do with it?

We could either pass in a value that will equal the same RD after calculation (seems to be `~190) and round the outcome, or just change the outcome of the test. I think I would prefer solution 1 but either is fine.

Edit: So I with solution 1 I meant setting the RD like this should yield the same results, maybe with a slight rounding error:

let player = GlickoRating {
    rating: 1500.0,
    deviation: (200.0_f64.powi(2) - 62.3_f64.powi(2)).sqrt(),
};

@asyncth
Copy link
Contributor Author

asyncth commented Dec 17, 2022

I'll do the first solution for test_glicko_rating_period, what about test_glicko?

@atomflunder
Copy link
Owner

I'll do the first solution for test_glicko_rating_period, what about test_glicko?

I think just changing the outcome is the best option there, since it is not based off of any official example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants