[ 
https://issues.apache.org/jira/browse/RNG-57?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16640712#comment-16640712
 ] 

Alex D Herbert commented on RNG-57:
-----------------------------------

I've closed the PR and repeated the changes in a new branch.

Do you want me to experiment with different seeds in {{ProvidersList}} until 
the tests pass? A quick change to add a few more numbers to the MWC_256 seed 
has fixed the test locally.

Unfortunately travis may still fail as it runs JDK 7, 8 and 9 and locally 
passing tests on one machine may not transfer.

I am wondering if the {{ProvidersList}} should make a better attempt at 
creating a full length seed. Perhaps this will make the test more robust.

Previously the RNGs that failed were:

||RNG||Native seed||Test seed||
|Well1024a|int[32]|int[3]|
|XorShift1024Star|long[16]|long[3]|
|MersenneTwister|int[624]|int[3]|
|MultiplyWithCarry256|int[257]|int[4]|

So the seed could be made full length for each provider.

Although each provider does expand the seed to the full length using a 
scrambling method the original seed could be improved using numbers that have 
more complete coverage of the bit size of the native seed type. The numbers 
typed into the test are quite short and may not contribute enough entropy.

Using java.util.Random is an option. However I am not sure the implementation 
is guaranteed to return the same sequence for the same seed for all future JDK 
versions. So hardcoding the seed is probably better.

WDYT?

- Repeat the PR as is with it failing and leave {{ProvidersList}} for a 
different change?
- Update the PR to include better seeds for {{ProvidersList}}

Applying the code changes is easy. I can create a temp branch to try different 
seeds until it passes on Travis then formulate an atomic PR.



> CachedUniformRandomProvider for nextBoolean() and nextInt()
> -----------------------------------------------------------
>
>                 Key: RNG-57
>                 URL: https://issues.apache.org/jira/browse/RNG-57
>             Project: Commons RNG
>          Issue Type: Improvement
>          Components: sampling
>    Affects Versions: 1.2
>            Reporter: Alex D Herbert
>            Priority: Minor
>              Labels: performance
>
> Implement a wrapper around a {{UniformRandomProvider}} that can cache the 
> underlying source of random bytes for use in the methods {{nextBoolean()}} 
> and {{nextInt()}} (in the case of {{LongProvider}}). E.g.
> {code:java}
> LongProvider provider = RandomSource.create(RandomSource.SPLIT_MIX_64);
> CachedLongProvider rng = new CachedLongProvider(provider);
> // Uses cached nextLong() 64 times
> rng.nextBoolean();
> // Uses cached nextLong() twice
> rng.nextInt();
> IntProvider provider = RandomSource.create(RandomSource.KISS);
> CachedIntProvider rng2 = new CachedIntProvider(provider);
> // Uses cached nextInt() 32 times
> rng2.nextBoolean();
> // This could be wrapped by a factory method:
> UniformRandomProvider rng = CachedUniformRandomProviderFactory.wrap(
>         // Any supported source: IntProvider or LongProvider
>         RandomSource.create(RandomSource...));
> {code}
> The implementation should be speed tested to determine the benefit for 
> {{nextBoolean()}} and if {{nextInt()}} can be improved for {{LongProviders}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to