8000 Fix - Use Fisher-Yates for uniform shuffling by aafulei · Pull Request #31 · glouw/tinn · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix - Use Fisher-Yates for uniform shuffling #31

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix - Use Fisher-Yates for uniform shuffling #31

wants to merge 1 commit into from

Conversation

aafulei
Copy link
@aafulei aafulei commented May 23, 2025

Hi @glouw first thank you very much for this elegant codebase 👍 It's truly like a piece of art to me!

However, a small issue with the shuffle algorithm: the current implementation does not produce a uniform random permutation. See here and Wikipedia: Fisher–Yates shuffle for reference.

I made a one-line change to improve with the standard Fisher-Yates algorithm, which should make the shuffling uniform. I hope this helps make the code even better. Thanks again for your great work!

Summary

This PR updates the shuffle function to implement the Fisher-Yates algorithm, ensuring a uniform random shuffle of the data.

Details

  • Changed the random index calculation to select from the current index a to the end of the array.
  • This fixes the bias in the previous shuffle implementation.
  • The change is minimal, modifying only one line for correctness.

Impact

  • Ensures all permutations are equally likely.
  • Improves reliability of training data shuffling in the neural network.

Please review and merge if everything looks good. Thanks!
@aafulei aafulei marked this pull request as ready for review May 23, 2025 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@aafulei
0