commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MATH-1300) BitsStreamGenerator#nextBytes(byte[]) is wrong
Date Sat, 19 Dec 2015 15:51:46 GMT

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

Gilles commented on MATH-1300:
------------------------------

Thanks for pointing out that problem.

bq. Sequential calls to \[...\] nextBytes must generate the same sequence of bytes, no matter
by chunks of which size it was divided.

Attempting to figure out how general the claim is, I've implemented a more general unit test
based on your suggestion:

{code}
    private void checkNexBytesChunks(int chunkSize,
                                     int numChunks) {
        final RandomGenerator rg = makeGenerator();
        final long seed = 1234567L;

        final byte[] b1 = new byte[chunkSize * numChunks];
        final byte[] b2 = new byte[chunkSize];

        // Generate the chunks in a single call.                                         
                                                                                         
                                          
        rg.setSeed(seed);
        rg.nextBytes(b1);

        // Reset.                                                                        
                                                                                         
                                          
        rg.setSeed(seed);
        // Generate the chunks in consecutive calls.                                     
                                                                                         
                                          
        for (int i = 0; i < numChunks; i++) {
            rg.nextBytes(b2);
        }

        // Store last 128 bytes chunk of b1 into b3.                                     
                                                                                         
                                          
        final byte[] b3 = new byte[chunkSize];
        System.arraycopy(b1, b1.length - b3.length, b3, 0, b3.length);

        // Sequence of calls must be the same.                                           
                                                                                         
                                          
        Assert.assertArrayEquals("chunkSize=" + chunkSize + " numChunks=" + numChunks,
                                 b2, b3);
    }
{code}

The original CM code always fails it, as you observed.
In your example test case (chunkSize=128 and numChunks=8), your fix makes the test pass.

However, it fails whenever the size of the array (argument to "nextBytes") is not a multiple
of 4.
That is, the "chunkSize" does matter.

And "nextBytes" in the JDK's {{Random}} class also fails the test when the array's size is
not a multiple of 4.

So there are several issues:
# Do you have a reference that the behaviour _must_ be as your described?
# Is the requested behaviour supposed to work only when the array's size is a multiple of
4?  If so, should we add some note about it in the documentation?
# Is there a way to implement "nextBytes" in {{BitsStreamGenerator}} so that the property
holds for any size?  And if so, should we consider  making the change (given that {{Random}}
does not work that way)?


> 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)

Mime
View raw message