Skip to content

Commit

Permalink
Is it possible we missed this for all these years?
Browse files Browse the repository at this point in the history 
@carothersc Can you look at this, Chris?  It was brought up by Jae-Seung at LLNL.
  • Loading branch information
@laprej
laprej committed Aug 19, 2015
1 parent 5b7d06c commit 45665b4
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion core/rand-clcg4.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ rng_init(int v, int w)
if(g_tw_rng_seed)
{
for(j = 0; j < 4; j++)
rng->seed[j] = *g_tw_rng_seed[j];
rng->seed[j] = g_tw_rng_seed[j];
} else
{
rng->seed[0] = 11111111;
Expand Down

6 comments on commit 45665b4

@carothersc-zz
Copy link
Member

@carothersc-zz carothersc-zz commented on 45665b4 Aug 19, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JohnPJenkins
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few notes/observations while I was looking up my changes from way back when:

  • I had made a few allocation path changes in the past, but didn't touch this part (see 119e16c)
  • we've never used the global seed in CODES (ie we always pass NULL into tw_define_lps). As discussed before, it's been discouraged to do our own manual clcg4 seeding and instead rely on the ROSS defaults.
  • LP RNGs don't interact with g_tw_rng_seed at all in the current setup, AFAICT - upon a call to tw_rand_init_streams->tw_rand_initial_seed->rng_init_generator, the SeedType argument is fixed at InitialSeed, which doesn't use the global rng that is initialized as a result of tw_pe_init->tw_rand_init->rng_init. I should note that none of the other seed types make an appearance in the ROSS codebase, so unless people are hand-crafting LP RNG structures, I have the feeling that this code hasn't been executed in a long time.

@gonsie
Copy link
Member

@gonsie gonsie commented on 45665b4 Aug 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @JohnPJenkins is right. The LP RNGs are totally separate from this. It looks like this is leftover code from where there was a per-MPI-rank RNG (or maybe a per-PE RNG). I don't think any of the models in carothersc/ROSS-Models use this, so it's probably safe to deprecate it.

@laprej
Copy link
Member Author

@laprej laprej commented on 45665b4 Aug 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the tw_rng_stream passed in to rng_init_generator() was set to values derived from rng->seed which in turn were set by rng_init() and g_tw_rng_seed will be used if it's non-zero.

@JohnPJenkins
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right - if you pass in a non-NULL seed array to tw_define_lps it will indeed get used to init the PE RNG. My bad. Does anyone do that though?

Also, your fix results in the following warning:

/homes/jenkins/work/ROSS/core/rand-clcg4.c:322:17: warning: assignment makes integer from pointer without a cast
    rng->seed[j] = g_tw_rng_seed[j];

Which makes sense since rng->seed is a an int32_t * whereas g_tw_rng_seed is a tw_seed * (= int32_t **). What was wrong with the previous version?

@JohnPJenkins
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's an relevant email on RNG seeding from Chris from a while back, coincidentally 2 days from a year ago :). Sounds like manual seeding (g_tw_rng_seed) should just be disabled entirely and the corresponding argument in tw_define_lps be ignored.

I think we stopped using that parameter since we did not want users picking
their own seeds as some seeds are better than others and some are just bad.

So, a little primer here on RNGs. RNGs in fact have only one (hopefully) very, very
long stream of random numbers. CLCG4 uses 4, 32bit values or 128 bits.
This generator has a period of 2^121 implying that ~2^7 values are missing
from the generator.This is fine.

So, you can image a circle with 2^121 tiny point going around the circumference
where each point would represent a random value. Eventually, if you called
the generator 2^121 times, you would have made it around the circle.

A very nice property of CLCG4 is that you can jump (or virtually teleport) ahead
in that single stream of random numbers. We call it "seed jumping". ROSS
starts from the default seed and jumps 2^70 calls ahead in the stream for each
LP. That is, each LP's starting point in the random stream circle is 2^72 calls
apart. So, in theory, we could support simulation with unto 2^49 LPs and each
LP have it's on part of the random stream, uncorrelated from other parts.

Now, to change seed sets, what you really want to do is change where each LP
starts on the random stream circle.

You can do this by increasing g_tw_nRNGs_per_lp. So, suppose you use 4.
That will give each LP 4 distinct RNG seed sets that start again 2^72 calls
apart.

In your model code, you just change which seed set you wish to us -- 0, 1, 2, 3
e.g., lp->rng[0] or lp->rng[1], lp->rng[2] or lp->rng[3].

By default in ROSS, when you use lp->rng your really saying &(lp->rng[0]).

The downside of this approach is you're potentially wasting some memory by
have these 4 seed sets around and the cost to compute them but it's a very
low cost all around unless you have a model with billions of LPs.

If that is the case, you would have to directly jump or shift seeds an LP uses
by directly invoking the seed jumping routine w/i the RNG with the LPs
init event.

e.g., call, tw_rand_initial_seed(&lp->rng[i], lp->gid + offset);
where offset would be the number of 2^70 jumps you want to make beyond the
LP's global id.

Please sign in to comment.