commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Herbert <alex.d.herb...@gmail.com>
Subject Re: [Rng] New XoShiRo generators
Date Wed, 20 Mar 2019 15:53:37 GMT
On 20/03/2019 11:14, Gilles Sadowski wrote:
>>
>> I have updated the PR for the XorShift1024 variant to deprecate the old XOR_SHIFT_1024_S
enum.
>>
>> Please take a look to see if the wording is OK:
>>
>> https://github.com/apache/commons-rng/pull/30/files#diff-630495e26d73fa22ada841dabde0ab47
<https://github.com/apache/commons-rng/pull/30/files#diff-630495e26d73fa22ada841dabde0ab47>
> How about:
>
> /**
>   * @deprecated Since 1.3, where it is recommended to use {@code
> XOR_SHIFT_1024_S_PHI},
>   * instead, due to its slightly better (more uniform) output.
>   * {@code XOR_SHIFT_1024_S} is still quite usable but both versions
> share the exact same
>   * internal state, so that their outputs are correlated (and thus
> should not be used together when
>   * independent sequences are assumed).
>   */
>
> ?

That is better as it states that the existing version is not broken and 
can still be used.

I've rearranged the text about sharing internal state as it could be 
misinterpreted since instances do not share state, only the method to 
maintain it. So I've changed it to variants of the same algorithm:

      * @deprecated Since 1.3, where it is recommended to use {@code 
XOR_SHIFT_1024_S_PHI}
      * instead due to its slightly better (more uniform) output. {@code 
XOR_SHIFT_1024_S}
      * is still quite usable but both are variants of the same 
algorithm and maintain their
      * internal state identically. Their outputs are correlated and the 
two should not be
      * used together when independent sequences are assumed.

I have missed off stating that they are correlated when identically 
seeded. When not identically seeded I presume it correct to say they are 
correlated given some defined lag? It's probably best to not mention it 
so a user does not think that using them together is OK with different 
seeds.

So I will remove the 'identically seeded' text from the class definition 
too and change it to:

  * <p>Note: This supersedes {@link XorShift1024Star}. The sequences 
emitted by both
  * generators are correlated.</p>
  *
  * <p>This generator differs only in the final multiplier (a 
fixed-point representation
  * of the golden ratio), which eliminates linear dependencies from one 
of the lowest
  * bits.</p>

I find the two paragraphs make it clearer they are correlated than if 
all in one paragraph.

I'll also add a note to XorShift1024Star that it has been superseded by:

  * <p>Note: This has been superseded by {@link XorShift1024StarPhi}. 
The sequences emitted
  * by both generators are correlated.</p>

Thus the recommendation is clear. Do not use them together to be assured 
of independence.

Alex




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message