Add tests to verify that our SplitMix/Seed avoids pathological γ-values #207
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thumphries
reviewed
Jul 2, 2018
| -- https://github.com/hedgehogqa/haskell-hedgehog/issues/191 | ||
| -- | ||
| prop_avoid_pathological_gamma_values :: Property | ||
| prop_avoid_pathological_gamma_values = |
There was a problem hiding this comment.
This looks fully deterministic to me, is that the case?
If so, we don't need to run this 100 times.
Suggest something like
withTests 1 . property $ do
for asserts $ \a ->
expected a === actual aThere was a problem hiding this comment.
Indeed, those are deterministic, so just doing the comparison once is enough. Good catch 👍 Fixed and force-pushed.
moodmosaic
force-pushed
the
topic/seed-tests
branch
from
b45c319
to
0f4f45c
Compare
|
Hmm, not sure why 1 of the 2 Travis CI checks fails. I think it's because I forced pushed the branch to the wrong origin, and then I forced pushed to the right one. |
As discussed by Melissa E. O'Neill in 'Bugs in SplitMix(es)' at http://www.pcg-random.org/posts/bugs-in-splitmix.html
thumphries
force-pushed
the
topic/seed-tests
branch
from
0f4f45c
to
1f14eee
Compare
|
Thanks! |
moodmosaic
commented
Jul 5, 2018
| data Assert = | ||
| Assert { | ||
| expected :: !Seed | ||
| , actual :: !Seed |
There was a problem hiding this comment.
Ugh! I forgot we're strict with no alignment anywhere (look at line 14). Happy to clean this up if you wish. /cc @jystic @thumphries
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We were missing the test part in Haskell, which is pretty similar to what we did in F#. See Melissa E. O'Neill's post with title Bugs in SplitMix(es), #191, and 39b15b9, for more context.
/cc @jystic @thumphries