-
Notifications
You must be signed in to change notification settings - Fork 177
Change the shuffle to the Fisher-Yates shuffle #72
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
Conversation
The current "Shuffling Array Elements" page recommends a naive algorithm, which produces a slow and biased shuffle. Included is a highly idiomatic refactorization of the Fisher- Yates shuffle (aka the Knuth Shuffle), a less-idiomatic-but-better refactorization, and a version of the algorithm that adds to Array.prototype, for those that program in that style.
|
THIS. This is what a fantastic recipe entry looks like. I saw this email and thought "oh no, they've deleted the simple shuffle and replaced it with a needlessly complex one". I came in here expecting to write a message asking you to preserve the old shuffle, but then explain when and why it might be unsuitable, and propose the Fisher-Yeates shuffle as a replacement. When I read your diff I wanted to cry with joy. d8uv, this is awesome. THANK YOU. This is the happiest "I hereby give you commit rights" message I've written to date. :-) Welcome! |
Change the shuffle to the Fisher-Yates shuffle
|
Actually, rereading it, might be better if it started with array.shuffle(), presented for fairly simple cases of randomness. Is it slightly biased, or is it obviously biased? I haven't run the Chi-squareq analysis on it to see how broken it is. I wonder if we could give some examples for when it works as a naive example, along with a specific use case that shows just how bad it can be--sort of like how you should never use modulus on Linear Congruential generators because rand() % 2 in the C stdlib will return 0,1,0,1,0,1,... etc. |
|
The chi-square analysis for the random.sort vs. fisher-yates has been done by Rob Weir. I haven't done a proper formal analysis of my refactorization, so if you're gonna drag out R you might as well check to see if my math-fu is strong, but my informal tests seem to pan out wonderfully. The reason I didn't write it in a "this is what everyone uses, these are the problems, and this is the solution" way is... I find that style to be a little complex when you're trying to learn something specific. People are here to learn how best to shuffle in Coffeescript, and Knuth Shuffle is the best way. People aren't here to learn about a bunch of different methods and their pitfalls, they just want the goods, and to get out. Adding that supplementary stuff at the end is great, and I'd be really curious as to what that'd look like, but the solution should stay at the top And as to if the naive solution is good enough... I'd say probably never. Doing it properly only takes up 5 lines of source, is a lot faster, and is free from bias. When you want something random, you really want it to be random, not random-ish. |
|
Heh, I misread the naive solution, sorry--I thought there was a Were there an array.shuffle() method, even if it was pretty bad (but not But yes. Doing a swapsort by random() is truly horrific--moving an Sorting by random DOES work with a schwartzian transform (assign each Thanks again! David On 02/28/2013 02:32 PM, d8uv wrote:
David Brady |
|
I... actually disagree with that. If |
The current "Shuffling Array Elements" page recommends a naive algorithm, which produces a slow and biased shuffle. Included is a highly idiomatic refactorization of the Fisher-Yates shuffle (aka the Knuth Shuffle), a less-idiomatic-but-better refactorization, and a version of the algorithm that adds to Array.prototype, for those that program in that style.