[
https://issues.apache.org/jira/browse/MATH-1300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15065423#comment-15065423
]
Gilles commented on MATH-1300:
------------------------------
Independently of the above discussion, I propose to replace the current code of
"nextBytes" with the following:
{code}
public void nextBytes(byte[] bytes) {
final int mask = 0xff;
final int numBytesChunk = 4;
final int shift = 8;
final int numBits = numBytesChunk * shift;
int index = 0;
int remainingBytes = bytes.length;
while (remainingBytes >= numBytesChunk) {
final int random = next(numBits);
for (int j = 0; j < numBytesChunk; j++) {
bytes[index++] = (byte) ((random >> (j * shift)) & mask);
--remainingBytes;
}
}
if (remainingBytes > 0) {
final int random = next(remainingBytes * shift);
for (int j = 0; j < remainingBytes; j++) {
bytes[index++] = (byte) ((random >> (j * shift)) & mask);
}
}
}
{code}
which I find much more legible (no unrolled loop, no repeated hard-coded
constants, explicit variables names).
It however changes the semantics in the part that fills the "remainingBytes",
where the argument to "next" requests only the required number of bits, thus
potentially (?) changing the bytes sequence. All the CM tests still pass, and
perhaps there should be a unit test that assert the expected semantics.
> BitsStreamGenerator#nextBytes(byte[]) is wrong
> ----------------------------------------------
>
> Key: MATH-1300
> URL: https://issues.apache.org/jira/browse/MATH-1300
> Project: Commons Math
> Issue Type: Bug
> Affects Versions: 3.5
> Reporter: Rostislav Krasny
> Attachments: MersenneTwister2.java, TestMersenneTwister.java
>
>
> Sequential calls to the BitsStreamGenerator#nextBytes(byte[]) must generate
> the same sequence of bytes, no matter by chunks of which size it was divided.
> This is also how java.util.Random#nextBytes(byte[]) works.
> When nextBytes(byte[]) is called with a bytes array of length multiple of 4
> it makes one unneeded call to next(int) method. This is wrong and produces an
> inconsistent behavior of classes like MersenneTwister.
> I made a new implementation of the BitsStreamGenerator#nextBytes(byte[]) see
> attached code.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
