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

Improve doc for Gen.pick #417

Merged
merged 1 commit into from
Dec 7, 2018
Merged

Conversation

ashawley
Copy link
Contributor

@ashawley ashawley commented Aug 10, 2018

This is in response to concerns raised about Gen.pick in #355.

Gen.pick, Gen.someOf, and Gen.atLeastOne will give a randomly chosen subset of elements, but not in a random order. This is because Gen.pick is implemented with reservoir sampling but without a shuffle operation. Adding shuffling might be possible, but it could impact performance and would violate the "do one thing well" principle of Gen.pick.

For now, just document the situation that results aren't permuted in the User Guide and in the API docs.

doc/UserGuide.md Outdated

Note that `Gen.someOf`, `Gen.atLeastOne`, and `Gen.pick` only randomly
select elements, they do not generate permutations of the selected
elements in different orders. Selection is random, but the order is
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Observed behavior of pick is that pick(2, 1 to 10) generates (for large samples) 73 distinct 2-lists, out of a total of 90 possible. 73 < 90 is consistent with the remark that pick does not, in general, permute all its results.

But since there are only 45 (=10*9/2) distinct 2-sets from (1 to 10), 73 > 45 implies that pick does permute some of its results, so the order is not deterministic. For instance List(4, 5) and List(5, 4) are both generated by pick(2, 1 to 10).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, using the word "deterministic" is confusing. I'll remove it.

Gen.pick, Gen.someOf, and Gen.atLeastOne will give a randomly chosen
subset of elements, but not in a random order.  This is because
Gen.pick is implemented with reservoir sampling but without a shuffle
operation.  Adding shuffling might be possible, but it could impact
performance and would violate the "do one thing well" principle of
Gen.pick.

For now, just document the situation that results aren't permuted in
the User Guide and in the API docs.
@ashawley
Copy link
Contributor Author

ashawley commented Oct 4, 2018

This issue came up again in the Gitter channel, yesterday.

The Scaladoc changes won't help right away, not until a new release is published, but the User Guide would be updated right away. The latter probably has a better chance of getting noticed.

@rickynils rickynils merged commit e6acf65 into typelevel:master Dec 7, 2018
@ashawley ashawley deleted the doc-shuffle-pick branch December 7, 2018 14:30
@ashawley ashawley mentioned this pull request Feb 11, 2019
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants