-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Align IterableDataset.shuffle with Dataset.shuffle #3842
Conversation
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
We should also add |
I added the I had to make a few other adjustments to make it work. In particular when you call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! One comment:
Co-authored-by: Mario Šaško <[email protected]>
From #3444 , Dataset.shuffle can have the same API than IterableDataset.shuffle (i.e. in streaming mode).
Currently you can pass an optional seed to both if you want, BUT currently IterableDataset.shuffle always requires a buffer_size, used for approximate shuffling. I propose using a reasonable default value (maybe 1000) instead.
In this PR, I set the default
buffer_size
value to 1,000, and I reorder theIterableDataset.shuffle
arguments to matchDataset.shuffle
, i.e. makingseed
the first argument.