jj1bdx / xorshiftplus

Archive of Sebastiano Vigna's xorshift+ and xorshift*
4 stars 0 forks source link

matrix initialisation bug in gentriples.c #2

Closed paul-ess closed 9 years ago

paul-ess commented 9 years ago

Firstly I would like to say that I found this code very useful for generating the triples for small xorshift generators in the range of [4-64] bits, so thanks for making your code available.

However after modifying it to work with smaller matrices I was getting some strange results, this turned out to be due to missing initialisation code in main(). I added the following to fix it:

    // Initialsie All Matrices to zero
    for( int i = 0; i < SIZE; i++ ) {
        for( int j = 0; j < SIZE; j++ ) {
            r[j].row[i] = 0ULL;
            l[j].row[i] = 0ULL;
        }
        id.row[i] = 0ULL;
    }

just before the call to identity().

For my compiler (g++./Cygwin) the r[1] matrix was garbage due to uninitialised memory.

jj1bdx commented 9 years ago

@paul-ess Would you send me a diff (or a pull request)? I can forward it to Sebastiano Vigna.

paul-ess commented 9 years ago

Hi Kenja,

I made quite a few other modifications to the code to make it generate triples for all widths in the rage [4-64] and I don’t think you want all of this stuff??

Also I’m quite new to GIT and not familiar with the collaborative features.

If I add only the initialisation fix to the original code and the email you a diff is that ok for you?

You are of course welcome to have the other updates if you think it is useful.

Regards,

Paul.

From: Kenji Rikitake [mailto:notifications@github.com] Sent: 27 May 2015 13:42 To: jj1bdx/xorshiftplus Cc: Paul Elliott Subject: Re: [xorshiftplus] matrix initialisation bug in gentriples.c (#2)

@paul-ess https://github.com/paul-ess Would you send me a diff (or a pull request)? I can forward it to Sebastiano Vigna.

— Reply to this email directly or view it on GitHub https://github.com/jj1bdx/xorshiftplus/issues/2#issuecomment-105892881 . https://github.com/notifications/beacon/AKtaZ8LfJ-tb4u4b5_m3uwcG1AKZCHeYks5oNbMGgaJpZM4Er1Gq.gif

jj1bdx commented 9 years ago

@paul-ess sending a diff for the necessary parts is certainly OK.

paul-ess commented 9 years ago

No problem, please see below:

$ diff gentriples.c gentriples_initfix.c

145a146,157

  // Initialsie All Matrices to zero - FIX PE/27/5/15

  // Note: For my compiler (g++/Cygwin) this was only found to be an issue when declaring smaller matrices

  // with size < 24. Basically the r[1] matrix looked like garbage.

  for( int i = 0; i < 64; i++ ) {

          for( int j = 0; j < 64; j++ ) {

                  r[j].row[i] = 0ULL;

                  l[j].row[i] = 0ULL;

          }

          id.row[i] = 0ULL;

  }

From: Kenji Rikitake [mailto:notifications@github.com] Sent: 27 May 2015 14:35 To: jj1bdx/xorshiftplus Cc: Paul Elliott Subject: Re: [xorshiftplus] matrix initialisation bug in gentriples.c (#2)

@paul-ess https://github.com/paul-ess sending a diff for the necessary parts is certainly OK.

— Reply to this email directly or view it on GitHub https://github.com/jj1bdx/xorshiftplus/issues/2#issuecomment-105910845 . https://github.com/notifications/beacon/AKtaZ73NfFzU6i0yY5MHgv80hxxFKKzeks5oNb-JgaJpZM4Er1Gq.gif

jj1bdx commented 9 years ago

Merged as https://github.com/jj1bdx/xorshiftplus/commit/0fece1801d0cc4da2c25ec5a263ccc14fe9bd3f9

Sorry for getting so late, but thanks!