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?

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Nick Sweet
  • 2,030
  • 3
  • 31
  • 48
  • 3
    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. –  Jun 13 '10 at 15:30
  • 3
    Well, that’s randomness: you can never be sure. (http://www.dilbert.com/strips/comic/2001-10-25/) ;-) – Gumbo Jun 13 '10 at 15:50
  • 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.) – Jonathan Leffler Jun 13 '10 at 15:53
  • 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?) – Ken Bloom Jun 13 '10 at 16:22
  • @Ken Bloom time_t is 8, unsigned is 4 and (unsigned)time(NULL) gives me values around "1276448696, 1276448742, 1276448748, 1276448753, 1276448761" – Nick Sweet Jun 13 '10 at 17:06
  • 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`. – Keith Thompson Aug 21 '14 at 18:12
  • 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? – Keith Thompson Aug 21 '14 at 18:16
  • @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. – Mooing Duck Aug 21 '14 at 18:29
  • @NickSweet: New theory: Are you using XCode? http://stackoverflow.com/questions/20263187/rand-14-only-generates-the-values-6-or-13?rq=1 – Mooing Duck Feb 20 '15 at 23:09
  • I was at the time, but that was XCode 4 years ago! – Nick Sweet Feb 21 '15 at 18:48

7 Answers7

9

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.

  • 2
    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? – Nick Sweet Jun 13 '10 at 17:00
  • 1
    @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". –  Jun 13 '10 at 17:20
  • 1
    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. – Mooing Duck Aug 21 '14 at 18:29
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;
Justin Ardini
  • 9,768
  • 2
  • 39
  • 46
  • 3
    While this is true, it doesn't answer the question at all. – mtvec Jun 13 '10 at 15:46
  • So, it is a one-cycle 'do while' loop - unnecessary but harmless. – Jonathan Leffler Jun 13 '10 at 15:48
  • 3
    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. – cHao Jun 13 '10 at 15:54
  • 2
    Your formula is wrong: it should be `randx = rand() % (max - min) + min`. – Matthieu M. Jun 13 '10 at 16:18
  • 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. – Nick Sweet Jun 13 '10 at 16:47
  • This is a bit of a hack, but if I add a line int temp = rand(); my problems go away! – Nick Sweet Jun 13 '10 at 16:50
  • @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. – Justin Ardini Jun 13 '10 at 17:16
  • 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! – Nick Sweet Jun 13 '10 at 17:24
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.

Waterybob
  • 49
  • 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`. – Keith Thompson Aug 21 '14 at 17:58
  • 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. – sonicbabbler Oct 03 '16 at 20:43
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;
M. Williams
  • 4,945
  • 2
  • 26
  • 27
2

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.

fredoverflow
  • 256,549
  • 94
  • 388
  • 662
  • @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. – fredoverflow Jun 13 '10 at 15:43
  • @Nick: Could you test this please and post if it works for you? – fredoverflow Jun 13 '10 at 15:53
  • 1
    This won't help at all. It will just shift the random number sequence three to the left. – Peter Alexander Jun 13 '10 at 16:40
  • 3
    @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! – Nick Sweet Jun 13 '10 at 17:02
  • 2
    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. – visitor Jun 14 '10 at 12:37
  • 3
    If your `rand()` implementation behaves this badly, you're probably better off not using it at all. – Keith Thompson Aug 21 '14 at 18:14
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.

Anonymous Coward
  • 19
  • 1
  • 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. – E.M. Jun 13 '10 at 16:55
  • 2
    Nope – the same number comes up for the first value to which I assign a random number each time I run the program. – Nick Sweet Jun 13 '10 at 16:57
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.

PYA
  • 8,096
  • 3
  • 21
  • 38
CaptainFox
  • 21
  • 1