rand() generating the same number – even with srand(time(NULL)) in my main!
Asked Answered
S

7

16

So, I'm trying to create a random vector (think geometry, not an expandable array), and every time I call my random vector function I get the same x value, though y and z are different.

int main () {
    srand ( (unsigned)time(NULL));
    Vector<double> a;
    a.randvec();
    cout << a << endl;
    return 0;
}

using the function

//random Vector
template <class T>
void Vector<T>::randvec()
{
    const int min=-10, max=10;
    int randx, randy, randz;

    const int bucket_size = RAND_MAX/(max-min);

    do randx = (rand()/bucket_size)+min;
    while (randx <= min && randx >= max);
    x = randx;

    do randy = (rand()/bucket_size)+min;
    while (randy <= min && randy >= max);
    y = randy;

    do randz = (rand()/bucket_size)+min;
    while (randz <= min && randz >= max);
    z = randz;
}

For some reason, randx will consistently return 8, whereas the other numbers seem to be following the (pseudo) randomness perfectly. However, if I put the call to define, say, randy before randx, randy will always return 8.

Why is my first random number always 8? Am I seeding incorrectly?

Salami answered 13/6, 2010 at 15:18 Comment(10)
Why not just do randx = (rand()/bucket_size)%max+min;? I don't think theres a need for a loop. If I'm wrong, I'm sorry, I just don't get it.Convulse
Well, that’s randomness: you can never be sure. (dilbert.com/strips/comic/2001-10-25) ;-)Schottische
The call to srand() is OK up until you want to be able to repeat a previous run - but that's a wholly separate problem from the 'persistent 8'. Maybe you should temporarily track the return values from rand() - perhaps with a wrapper function. And I'd be worried about the repetition of the algorithm; use a function 'int randominteger(int min, int max)' to avoid it. (Note that seeding with time is not good for cryptographic randomness - it is rather predictable.)Cyruscyst
What's sizeof(time_t) on your system? What's sizeof(unsigned)? Does (unsigned)time(NULL) return the same value repeatedly? (Does the value get truncated wierdly?)Scyphus
@Ken Bloom time_t is 8, unsigned is 4 and (unsigned)time(NULL) gives me values around "1276448696, 1276448742, 1276448748, 1276448753, 1276448761"Salami
The cast is unnecessary (but harmless). srand(time(NULL)) will implicitly convert the time_t result of time(NULL) to the unsigned argument type required by srand.Montreal
The code in the question is incomplete. You should be able to narrow it down to a complete program that just calls srand and rand and displays the results. If it produces the same anomaly, that will make it easier to track down. If it doesn't, perhaps there's something wrong in the implementation of your Vector class. Some implementations of rand() are better behaved than others. What OS and runtime library are you using?Montreal
@NickSweet: make a new project, copy the code from this question to it, and run it. Does the problem still occur? The results you describe make me think there's another srand you forgot about somewhere.Object
@NickSweet: New theory: Are you using XCode? #20263687Object
I was at the time, but that was XCode 4 years ago!Salami
N
10

The issue is that the random number generator is being seeded with a values that are very close together - each run of the program only changes the return value of time() by a small amount - maybe 1 second, maybe even none! The rather poor standard random number generator then uses these similar seed values to generate apparently identical initial random numbers. Basically, you need a better initial seed generator than time() and a better random number generator than rand().

The actual looping algorithm used is I think lifted from Accelerated C++ and is intended to produce a better spread of numbers over the required range than say using the mod operator would. But it can't compensate for always being (effectively) given the same seed.

Nudge answered 13/6, 2010 at 16:0 Comment(3)
You're right – it's straight from Accelerated C++. However, I'm not sure you're right about the seeded values being that close together. I'm running the program many times in the same minute (say 10 times in 20 seconds) and each time I still get 8 as my first random number. The first numbers I'm getting from rand each time I run it are: 2046680223610024096 20471340121436194148 2047251661855022070 20473356961746421126 20474533451699427348 20475205731980895774 20476046081258989483 That seems to be doing its job, right?Salami
@Nick The numnbers are different, but they are not VERY different, which is what the algorithm from Accelerated C++ requires. It seems unlikely that if you choose 10 genuinely random numbers they will all begin with the digits "20".Nudge
I don't think so, the parameter to srand ought to act like a hash, giving values different by 1 should be enough to give different first values.Object
F
5

I don't see any problem with your srand(), and when I tried running extremely similar code, I did not repeatedly get the same number with the first rand(). However, I did notice another possible issue.

do randx = (rand()/bucket_size)+min;
while (randx <= min && randx >= max);

This line probably does not do what you intended. As long as min < max (and it always should be), it's impossible for randx to be both less than or equal to min and greater than or equal to max. Plus, you don't need to loop at all. Instead, you can get a value in between min and max using:

randx = rand() % (max - min) + min;
Forthcoming answered 13/6, 2010 at 15:34 Comment(8)
While this is true, it doesn't answer the question at all.Lynlyncean
So, it is a one-cycle 'do while' loop - unnecessary but harmless.Cyruscyst
Harmless except that it doesn't do what it's expected to -- and someone glancing over the code (and not looking too closely) will see it checking bounds, when it actually isn't.Hanny
Your formula is wrong: it should be randx = rand() % (max - min) + min.Pair
I have to say, I'm a little confused – I took the do while loop straight out of Accelerated C++ by Andrew Koenig and Barbara E. Moo. They give this reason: rand() really returns only pseudo-random numbers. Many C++ implementations' pseudo-random-number generators give remainders that aren't very random when the quotients are small integers. For example, it is not uncommon for successive results of rand() to be alternately even and odd. In that case, if n is 2, successive results of rand() % n will alternate between 0 and 1. I tried the modulo version, and now '0' become my x value each time.Salami
This is a bit of a hack, but if I add a line int temp = rand(); my problems go away!Salami
@Nick: Are you sure Accelerated C++ didn't use || instead of && for the condition of the do-while loops? I'm no expert on the statistics of pseudo-random number generation, but if you are willing to use Boost, you can instead use Boost.Random to generate pseudorandom numbers within ranges.Forthcoming
Nope, you're completely right – it's a || rather than a &&. I just didn't mention that I'd changed that due to the post character limit, but I've changed that and it makes logical sense now. Thanks!Salami
N
4

I had the same problem exactly. I fixed it by moving the srand() call so it was only called once in my program (previously I had been seeding it at the top of a function call). Don't really understand the technicalities - but it was problem solved.

Numerable answered 22/2, 2012 at 14:32 Comment(2)
Yes, srand() should be called exactly once, before any calls to rand(). As it happens, that's exactly what the code in the question does; there's exactly one call to srand() at the top of main.Montreal
Thank you. I srand((unsigned)time(NULL)) within a loop and getting numbers that were too similar. Once I moved outside of the loop it worked much better.Seaside
G
3

A simple quickfix is to call rand a few times after seeding.

int main ()
{
    srand ( (unsigned)time(NULL));
    rand(); rand(); rand();

    Vector<double> a;
    a.randvec();
    cout << a << endl;
    return 0;
}

Just to explain better, the first call to rand() in four sequential runs of a test program gave the following output:

27592
27595
27598
27602

Notice how similar they are? For example, if you divide rand() by 100, you will get the same number 3 times in a row. Now take a look at the second result of rand() in four sequential runs:

11520
22268
248
10997

This looks much better, doesn't it? I really don't see any reason for the downvotes.

Guillemette answered 13/6, 2010 at 15:26 Comment(7)
@pax: What is there not to understand? If the first n numbers from rand() are always the same, you can simply discard those first n numbers by calling rand() n times and ignoring the result.Guillemette
@Nick: Could you test this please and post if it works for you?Guillemette
This won't help at all. It will just shift the random number sequence three to the left.Afebrile
@FredOverflow This works, but it's a bit of a hack and doesn't really figure out what's going on – it just deals with it. If I can't figure this problem out, I'll use your technique, but I'd rather get to the heart of the issue. Thanks, though!Salami
It is a bit woodoo programming but the advice doesn't seem that bad. If the first value you get is too similar but the rest are OK, then throw away the first value(s). The idea of rand is that completely different sequences emerge from seeds that are rather similar.Satiety
If your rand() implementation behaves this badly, you're probably better off not using it at all.Montreal
@KeithThompson yes, rand() is obsolete and there are lots of bad implementations out there. One of the worst is provided in Microsoft's standard library. You're much better off using the <random> header introduced in C++11.Yarber
G
3

Also to mention, you can even get rid of that strange bucket_size variable and use the following method to generate numbers from a to b inclusively:

srand ((unsigned)time(NULL));

const int a = -1;
const int b = 1;

int x = rand() % ((b - a) + 1) + a;
int y = rand() % ((b - a) + 1) + a;
int z = rand() % ((b - a) + 1) + a;
Gloxinia answered 13/6, 2010 at 15:33 Comment(2)
Redundant cast in srand call.Coracorabel
@user562374: Depends on your warning level. MSVC emits warnings if the cast is missing.Object
M
1

Your implementation, through integer division, ignores the smallest 4-5 bit of the random number. Since your RNG is seeded with the system time, the first value you get out of it will change only (on average) every 20 seconds.

This should work:

randx = (min) + (int) ((max - min) * rand() / (RAND_MAX + 1.0));

where

rand() / (RAND_MAX + 1.0)

is a random double value in [0, 1) and the rest is just shifting it around.

Mv answered 13/6, 2010 at 16:17 Comment(2)
I think you correct, but when I tried your code it produced similar problems to the OP's. If I remove the cast to int I see variation in the 2nd decimal place. I assume that if I waited long enough between runs, I'd see randx change, but that could be years at this rate.Christiansand
Nope – the same number comes up for the first value to which I assign a random number each time I run the program.Salami
J
0

Not directly related to the code in this question, but I had same issue with using srand ((unsigned)time(NULL)) and still having same sequence of values being returned from following calls to rand().

It turned out that srand needs to called on each thread you are using it on separately. I had a loading thread that was generating random content (that wasn't random cuz of the seed issue). I had just using srand in the main thread and not the loading thread. So added another srand ((unsigned)time(NULL)) to start of loading thread fixed this issue.

Javelin answered 5/12, 2018 at 12:14 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.