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

Rate command #81

Merged
merged 11 commits into from
Mar 6, 2017
Merged

Rate command #81

merged 11 commits into from
Mar 6, 2017

Conversation

amj
Copy link
Member

@amj amj commented Jan 2, 2017

Further refactors & cleanup from your previous comments. Runs all good...

Next steps are refactoring to save a little state on repeated runs -- would be useful for bootstrapping/testing as we try and find the proper values of these parameters.

Copy link
Contributor

@brilee brilee left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me. I haven't actually gotten around to executing this yet, since I don't have the actual game data dump.

@@ -0,0 +1,22 @@


def rating_to_traditional_range(rating_f):
Copy link
Contributor

Choose a reason for hiding this comment

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

Does "traditional range" actually exist? My impression of AGA raw ratings was that all positive ratings were "rounded up" so that [0, 1] = 1dan, whereas negative ratings were rounded down so that [-1, 0] = 1kyu.

Copy link
Member Author

Choose a reason for hiding this comment

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

the 'kyu-dan boundary' is between [-1,1] -- you can see how they're adjusted in the current regime here: https://github.com/usgo/AGA-Ratings-Program/blob/master/collection.cpp#L356.

The math on that one is a little different; IIRC, it maybe enforces a lower bound of -30, but there is no upper bound enforced in the math. The ratings that expand up past 7.0 are only anchored by the fact that there's a lot of priors, IIUC

Copy link
Member Author

Choose a reason for hiding this comment

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

(i'm pretty happy about the fact that, with ZERO priors, zero anchors, zero thumbs-on-the-scale, we end up with a really close match on a couple dozen different players i looked up. Would love to turn that into a quantifiable thing, but i'm really happy so far)

@@ -36,44 +34,45 @@ def sanitized_games(games):
elif not (g.result.startswith('W') or g.result.startswith('B')):
print('unknown result: ', g)
pass
elif g.date_played is None or g.date_played.timestamp() == 0.0:
print('No date played: ', g)
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

I was confused by pass, since the intent is to skip this game. I'd use continue everywhere just for consistency, even though code-wise it executes identically as pass in this case because the final action is in an else branch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, will do.

@@ -100,8 +99,10 @@ def rate_all(t_from=None, t_to=None, iters=200, lam=.22):
for k,v in rating_prior.items():
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this sensitive to outliers? If you got one unusually bad or good player, would everyone else end up with squished ratings? Maybe it'd be more robust to use the mean and std deviation and rescale that way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, this seems like the kind of renormalization that should happen after the algorithm is finished, but I can also see why it would help stabilize the iterations within the loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

re: outliers it's certainly possible. I didn't test any crazy outliers on the bottom side -- on the top side, it seems to do 'the right thing' on the AGA's set, which has some undefeated players on the top like Rui Naiwei, Myungwan Kim, etc.

re: renormalization: I'm not sure. I added this when i was still having some non-convergence issues, but it might not still be needed.

elif rating < 7:
return "%dd" % int(rating)
else:
return "7d"
Copy link
Contributor

Choose a reason for hiding this comment

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

AlphaGo can only be 7d AGA :D

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah this might need to be something else later ;)

Copy link
Member

Choose a reason for hiding this comment

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

I thought the AGA ratings went up to 9D?

Copy link
Member Author

Choose a reason for hiding this comment

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

"ratings" go up to 11ish. Ranks only go to 7d.

@artasparks
Copy link
Member

Is this PR ready to be merged?

@amj
Copy link
Member Author

amj commented Mar 6, 2017

hmm, i think so. Let me give it a quick pass

@amj amj merged commit f882153 into master Mar 6, 2017
@brilee brilee deleted the rate-command branch March 6, 2017 14:57
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.

3 participants