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

random: don't bias towards earlier users #97

Merged
merged 2 commits into from Feb 4, 2019

Conversation

Projects
None yet
4 participants
@jeffkaufman
Copy link
Owner

jeffkaufman commented Feb 4, 2019

Currently our random selection has a bias towards earlier users. For each user we generate a random number and see if their total is below it, so there's an edge to being earlier. This edge gets larger as points grow. For example, if we had two players each with 50k points, the first player would win 50% of the time, while the second would only win 50% of remainder, or 25% of the time.

Switch to picking a single random number and seeing which, if any, player that corresponds to. Now both players would win 50% of the time.

Testing this, with the points scalar at 0.001 instead of 0.00001 to not take so long, I see:

   8 Exception: pavellishin wins!
  10 Exception: csvoss wins!
  13 Exception: jeffkaufman wins!
  14 Exception: tnelling wins!
 955 The game continues.

which seems reasonable given our current points.

random: don't bias towards earlier users
Currently our random selection has a bias towards earlier users.  For each user we generate a random number and see if their total is below it, so there's an edge to being earlier.  This edge gets larger as points grow.  For example, if we had two players each with 50k points, the first player would win 50% of the time, while the second would only win 50% of remainder, or 25% of the time.

Switch to picking a single random number and seeing which, if any, player that corresponds to.

Testing this with the points scalar at 0.001 instead of 0.00001 to not take so long, I see:

   8 Exception: pavellishin wins!
  10 Exception: csvoss wins!
  13 Exception: jeffkaufman wins!
  14 Exception: tnelling wins!
 955 The game continues.

which seems reasonable given our current points.

@jeffkaufman jeffkaufman added the reviewme label Feb 4, 2019

@jeffkaufman jeffkaufman requested review from csvoss , pavellishin and tnelling Feb 4, 2019

jeffkaufman added a commit that referenced this pull request Feb 4, 2019

start using reproducible random numbers
Currently we have Travis running random.random() to determine if someone has won.  This of course gives different values each time you run it.  This is basically fine, but it would be better to use a hash of something that (a) we all can verify and (b) we can't predict.  Then we don't have to trust Travis or worry about people re-running Travis builds.

This PR switches us to using the hash of the merge commit as our source of randomness.  We only get one of these, so this depends on the fix in #97.
Show resolved Hide resolved validate.py
@pavellishin

This comment has been minimized.

Copy link
Collaborator

pavellishin commented Feb 4, 2019

I love the comment and illustration, does a great job illustrating the intent.

@pavellishin
Copy link
Collaborator

pavellishin left a comment

I think the point summation is wrong.

@pavellishin

This comment has been minimized.

Copy link
Collaborator

pavellishin commented Feb 4, 2019

Also, what's the current state of negative points? Do we explicitly fail if a transfer results in negative points?

@jeffkaufman

This comment has been minimized.

Copy link
Owner Author

jeffkaufman commented Feb 4, 2019

@pavellishin

Also, what's the current state of negative points? Do we explicitly fail if a transfer results in negative points?

Yes, rules/0.2-block-negative-points.py prohibits any PR merging if any user would end up with negative points.

@pavellishin

This comment has been minimized.

Copy link
Collaborator

pavellishin commented Feb 4, 2019

This change is pretty interesting, because it also means that once 100,000 points have been accumulated across all users, a winner is guaranteed.

@tnelling

This comment has been minimized.

Copy link
Collaborator

tnelling commented Feb 4, 2019

This change is pretty interesting, because it also means that once 100,000 points have been accumulated across all users, a winner is guaranteed.

In addition to guaranteeing a winner at a certain point, it also generally and significantly increases the odds that someone wins. Though this is hypothetically offset by #98 preventing infinite re-rolls.

@csvoss

csvoss approved these changes Feb 4, 2019

@pavellishin

This comment has been minimized.

Copy link
Collaborator

pavellishin commented Feb 4, 2019

it also generally and significantly increases the odds that someone wins

It does? Shouldn't it actually decrease the odds, since we're not trying once per user now?

@tnelling

This comment has been minimized.

Copy link
Collaborator

tnelling commented Feb 4, 2019

it also generally and significantly increases the odds that someone wins

It does? Shouldn't it actually decrease the odds, since we're not trying once per user now?

Consider the case where two users have 50% of the max value. Under the proposed system, as you've noted, someone is guaranteed to win. Under the current system, there's only a 75% chance someone wins. The odds are multiplicative under the current system, but additive under the proposed system.

@pavellishin

This comment has been minimized.

Copy link
Collaborator

pavellishin commented Feb 4, 2019

Oh, you're right. I thought that with smaller point values, it would be different, but two users that have 10% of the points each would mean a 19% and a 20% chance of winning under the old and new system, respectively. (If I did my math right.)

@tnelling

This comment has been minimized.

Copy link
Collaborator

tnelling commented Feb 4, 2019

Yeah. They do get asymptotically close at small values (e.g. at 1% chance each it's 1.99% and 2% respectively) also get closer together as the player values diverge from each other, so the word "significantly" wasn't really accurate in all cases in what I said.

@jeffkaufman

This comment has been minimized.

Copy link
Owner Author

jeffkaufman commented Feb 4, 2019

It increases the odds, but only by bringing them from their artificially low place to where they should be.

@jeffkaufman

This comment has been minimized.

Copy link
Owner Author

jeffkaufman commented Feb 4, 2019

@pavellishin needs re-review since your earlier rejection

@jeffkaufman jeffkaufman merged commit 0c08b50 into master Feb 4, 2019

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

@jeffkaufman jeffkaufman deleted the unbiased-random branch Feb 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment