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

Expand the ranges of arbitrary integers. #221

Closed
wants to merge 1 commit into from

Conversation

romanb
Copy link

@romanb romanb commented Nov 30, 2018

This is supposed to address both #119 and #190 by interpreting the generator's size parameter as a bias towards the induced range, instead of as a hard limit. Specifically:

  • Values within the range induced by the size parameter occur with > 0.5 probability (currently ~.75 but it needn't be exposed to such a granularity on the API, so as to allow easy modifications / refinement).
  • Values outside of the induced range occur with < 0.5 probability.
  • Special values like 0, min_value and max_value generally occur more frequently than any other value.

In accordance with intuition, lowering the size parameter increases the probability of smaller numbers and increasing the size parameter increases the probability of larger numbers being generated.

The distribution could be further refined but I was trying to keep it simple while solving the main problem of sampling from the entire range such that the influence of the generator's size is easily described and understood. Here are some sample distributions with size=100 (the default size).

Further comments:

  1. Because I expanded the tests a little and also wanted to cover i128 / u128, I bumped the dependencies on rand and rand_core to 0.6 and 0.3, respectively, requiring a few minor modifications.

  2. Because the latest version of the transitive dependency lazy_static has a lower bound for rustc >= 1.24.1 I bumped the lower bound for quickcheck accordingly. If you are willing to go up to 1.26, the feature gate for i128 could be removed, as it stabilised in that release. Would that be desirable?

  3. The assert!s serve only to state the established invariants and could be removed or turned into comments, if desired.

@BurntSushi
Copy link
Owner

@romanb Would you might rebasing this? I took care of the rand/rand_core upgrade and the MSRV tweak in separate commits.

@romanb
Copy link
Author

romanb commented Jan 5, 2019

@BurntSushi Done. Since the MSRV is now 1.30, I could also remove the conditional compilation for the i128 feature as part of this PR, if desired. Nevermind, you did that already!

Interpret the generator's size parameter as a bias towards the induced
range, instead of as a hard limit. Specifically:

  * Values within the range induced by the size parameter occur with >
    0.5 probability.
  * Values outside of the induced range occur with < 0.5 probability.
  * Special values like 0, min_value and max_value generally occur
    more frequently than any other value.

In accordance with intuition, lowering the size parameter increases the
probability of smaller numbers and increasing the size parameter
increases the probability of larger numbers being generated.
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.

2 participants