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

Change the random seed generation to use milliseconds #121

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@rlee287
Copy link

rlee287 commented Oct 30, 2018

Running games multiple times a second will result in seed reuse. This issue came up when I was testing using the Halite III gym and getting "runs" of wins that were much longer than statistically expected. This PR fixes this issue by using milliseconds as the seed parameter.

Change the random seed generation to use milliseconds
Running games multiple times a second will result in seed reuse
@Janzert

This comment has been minimized.

Copy link
Collaborator

Janzert commented Oct 30, 2018

This narrows the time period for a seed collision from 1 second to 1 millisecond. That should stop the problem for most normal usage. But could still exhibit problems in rare situations, such as when all but one bot happens to crash at game start. Also it's been mentioned in discord that the gym can be modified fairly easily for parallel execution which could also run into multiple games starting within the same millisecond.

So instead of, or along side, this change it might be best to modify the gym to supply explicit seeds when launching the engine. That should be much easier to ensure they are unique.

Also a side note, and I may very well be the only one that has this, I have local scripts that rely on the seed for server games being the unix timestamp at the start of the game. But that is still easily reconstructed after this change.

@snaar

This comment has been minimized.

Copy link
Contributor

snaar commented Oct 30, 2018

I also have mild preference for asking user to supply seed themselves in cases when they care about it. I.e. I actually think it's a feature that seed is "bad" right now. It works in most simple cases, and if you are doing something complicated then you should really supply the seed to the engine yourself. So +1 on modifying the gym instead.

@fqlx

This comment has been minimized.

Copy link

fqlx commented Nov 19, 2018

@snaar humans are bad at picking random numbers

@rlee287

This comment has been minimized.

Copy link

rlee287 commented Nov 19, 2018

It will be up to the Two Sigma team whether or not to merge this in but I solved this problem by having a different random number generator in my gym script that did not have this seeding time resolution issue.

@snaar

This comment has been minimized.

Copy link
Contributor

snaar commented Nov 22, 2018

@fqlx this wouldn't be humans, this would be external programs that launch game in bulk. I can easily see cases where milliseconds is actually insufficient as well. This is simply good design where if you need guarantee that something is unique, you do in a place that can actually guarantee it. The engine cannot do it, since two instances of engine cannot communicate with each other.

@snaar

This comment has been minimized.

Copy link
Contributor

snaar commented Nov 22, 2018

In fact, @rlee287 solved it in the the way that I would prefer it solved by editing the gym instead.

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