Seeded Random Shuffle #1110
Merged
Seeded Random Shuffle #1110
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
In order to extend this and use it in other places than just the coin selection it makes sense to give it a proper home.
We don't need a crypto-resistent generator here and a system generator will do for shuffling outputs or stake pools
This would be pretty useful to bridge use any entity (e.g. WalletId) with a ToText instance as a seed for such generator
2 tasks
Anviking
approved these changes
Dec 3, 2019
lib/core/src/Data/Vector/Shuffle.hs
Outdated
| @@ -24,7 +24,7 @@ shuffle :: [a] -> IO [a] | |||
| shuffle = modifyInPlace $ \v -> do | |||
| let (lo, hi) = (0, MV.length v - 1) | |||
| forM_ [lo .. hi] $ \i -> do | |||
| j <- fromInteger <$> generateBetween (fromIntegral lo) (fromIntegral hi) | |||
| j <- fromInteger <$> randomRIO (fromIntegral lo, fromIntegral hi) | |||
There was a problem hiding this comment.
Both take closed intervals, so should be the same 👍
| -- ∃(Δ: Int). | ||
| -- ∀(es :: [a]). | ||
| -- | ||
| -- g0 ≠g1, length es > Δ⇒ shuffleWith g0 es ≠shuffleWith g1 es |
There was a problem hiding this comment.
Nit-pick / confusion: the ∃(Δ: Int) part sounds 1) questionable to me, and 2) not the same as the code?. Whatever ∆, given enough time, this property is always disprovable?
There was a problem hiding this comment.
Well, to be honest, I was hoping to have the property to fail and have to find a threshold manually (even though this is entirely subjective to the QC generator).
Still, it's hard to translate in pure maths here because that's really a probabilistic results. We can't possibly guarantee that this is true for any list and any seed. But, most of the time, it should be a reasonable assumption. In particular, when lists are big enough (and it's this is what the threshold translates), it should hold with a reasonable confidence.
Turns out that, 90% of the results satisfies the property already with a threshold of 0 🤷♂️, QC isn't generating that many singletons.
|
bors r+ |
iohk-bors bot
added a commit
that referenced
this pull request
Dec 3, 2019
1110: Seeded Random Shuffle r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> #1102 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - 995e006 move 'shuffle' (and specs) into a separate module In order to extend this and use it in other places than just the coin selection it makes sense to give it a proper home. - 9064a78 replace 'Crypto.Number.Generate' by 'System.Random' We don't need a crypto-resistent generator here and a system generator will do for shuffling outputs or stake pools - 5a7bb37 allow shuffle to be seeded with a 'RandomGen' - 7c95cec implement 'mkSeed' to generate a seed from a text This would be pretty useful to bridge use any entity (e.g. WalletId) with a ToText instance as a seed for such generator # Comments <!-- Additional comments or screenshots to attach if any --> ``` Data.Vector.Shuffle shuffle every non-empty list can be shuffled, ultimately +++ OK, passed 100 tests (92% shuffled). shuffle is non-deterministic +++ OK, passed 100 tests (92% not deterministic). sort (shuffled xs) == sort xs +++ OK, passed 100 tests (93% non-empty). shuffleWith / mkSeed shuffling with the same seed is deterministic +++ OK, passed 100 tests (94% non singleton). different seed means different shuffles +++ OK, passed 200 tests; 21 discarded: 91.0% different 66.0% big list 28.0% small list 6.0% singleton ``` <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <[email protected]>
Build succeeded |
KtorZ
added this to the (WB-34) Deal with bootstrapping of the `apparent_performance` for stake pools milestone
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.
Issue Number
#1102
Overview
995e006 move 'shuffle' (and specs) into a separate module
In order to extend this and use it in other places than just the coin selection
it makes sense to give it a proper home.
9064a78 replace 'Crypto.Number.Generate' by 'System.Random'
We don't need a crypto-resistent generator here and a system generator
will do for shuffling outputs or stake pools
5a7bb37 allow shuffle to be seeded with a 'RandomGen'
7c95cec implement 'mkSeed' to generate a seed from a text
This would be pretty useful to bridge use any entity (e.g. WalletId) with a
ToText instance as a seed for such generator
Comments