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

Better randomization, especially for GUID generation #99

Closed
jeffrose opened this issue Nov 15, 2012 · 8 comments
Closed

Better randomization, especially for GUID generation #99

jeffrose opened this issue Nov 15, 2012 · 8 comments

Comments

@jeffrose
Copy link

In general you should not depend on Math.random() for anything related to GUID generation. This page explains the issue in detail as well provides JavaScript algorithms that provide better randomization. Consider adopting one those, likely Kybos since it is clearly licensed.

@millermedeiros
Copy link
Owner

I agree that Math.random() isn't good enough and that a seeded random is extremely useful. I will take a look on the document and implementations and check out if we can use it or not.

@bebraw
Copy link

bebraw commented Nov 22, 2012

Do you think Mersenne Twister would do the trick? I used that once to get repeatable random numbers (you can set the seed). Perhaps it will work for your purposes too.

@jeffrose
Copy link
Author

The page I referenced mentions Mersenne Twister a couple times and precludes using it since it fails Crush randomness tests.

@millermedeiros
Copy link
Owner

I was thinking about it and I fell that providing a single algorithm is not going to be a good idea and is also out of the scope of this project (besides the fact that it is really hard to test PRNG properly).

I think we should add a way to swap the PRNG used by each method so you could curry each method individually and use different seeds for each one:

var mt = new MersenneTwister('magic beans seed');
var mtRandom = function(){ return mt.random() };
var randInt_1 = randInt.create( mtRandom );
var randInt_2 = randInt.create( KISS07('sunflower seed') );

And also add a way to replace it on an existing instance:

// default value = Math.random;
randInt.random = Kybos('foobar');

And maybe create a new module random/random that is used by all the methods inside the random package (avoid calling Math.random() directly), so replacing it would be enough to propagate the change to all random methods:

define(function(){
   function random(){
     return random.random();
   }
   // expose a `random` property so user can swap the behavior dynamically
   random.random = Math.random;
   return random;
});

Could also use the RequireJS map config to replace the amd-utils/random/random module, but I like the idea of exposing the random property since on node.js there is no such a thing as a map config.

PS: that way we also avoid any licensing issues.

@jeffrose
Copy link
Author

I definitely like the flexibility that having a separate module, random/random, provides. I assume the default implementation will still be Math.random(). There should probably be some comment about the unreliability of Math.random() especially for UUID generation.

Having said that random/random looks a little funny. Maybe random/gen? random/generator? random/ng? random/prng?

@millermedeiros
Copy link
Owner

I added the random/random, since I think it will only be used internally (easier to call Math.random directly) I think the name is good enough.

I didn't implemented the randInt.create(), choice.create(), etc.. or added a new property random to each method since I think it will increase complexity a lot and the global replacement should be enough for most use cases (overriding amd-utils/random/random behavior).

I improved the documentation a lot adding info on the top about Math.random usage and also improved the guid documentation to mention safety problems. random/random gets into a lot of details so it should be clear to everyone: http://millermedeiros.github.com/amd-utils/random.html

Thanks for all the feedback and for helping me to improve amd-utils. Feel free to reopen the issue.

@millermedeiros
Copy link
Owner

for future reference this article is great: http://www.empiricalzeal.com/2012/12/21/what-does-randomness-look-like/

@jeffrose
Copy link
Author

Thanks! I'll check it out.

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

No branches or pull requests

3 participants