Random Numbers -- why doesn't this piece of code work?

[Moritz Beller]

Dear programmers,

Have a look at this function to generate random numbers from 1 to count.
It simply does not work. Well, actually it compiles silently, but then
returns "1" (Compiler: gcc/++ 3.3.2-r5; Gentoo Linux).

#include <cstdlib> // For random number generator
#include <ctime> // For time function
using namespace std;

inline int random(int count) {
return 1 +
static_cast<int>((count*static_cast<long>(rand()))/(RAND_MAX+1));
}

I was able to find out that the command _will_ work as it is expected
with out RAND_MAX, but obviously doesn't make any sense, then.

Thanks in advance,
Moritz Beller

 

[Leor Zolman]

Dear programmers,

Have a look at this function to generate random numbers from 1 to count.
It simply does not work. Well, actually it compiles silently, but then
returns "1" (Compiler: gcc/++ 3.3.2-r5; Gentoo Linux).

#include <cstdlib> // For random number generator
#include <ctime> // For time function
using namespace std;

inline int random(int count) {
return 1 +
static_cast<int>((count*static_cast<long>(rand()))/(RAND_MAX+1));
}

I was able to find out that the command _will_ work as it is expected
with out RAND_MAX, but obviously doesn't make any sense, then.

Thanks in advance,
Moritz Beller

I haven't thought about your logic, because, given:

#include <iostream>
#include <cstdlib> // For random number generator
#include <ctime> // For time function
using std::cout;
using std::endl;

inline int random(int count) {
return 1 +

static_cast<int>((count*static_cast<long>(rand()))/(RAND_MAX+1));
}

int main()
{
srand(time(0));
cout << random(10) << endl;
cout << random(100) << endl;
cout << random(1000) << endl;
cout << random(1000) << endl;

return 0;
}


Here's what transpires:

d:\src\learn>g++ --version
g++ (GCC) 3.2.3 (mingw special 20030504-1)
Copyright (C) 2002 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

d:\src\learn>g++ rand.cpp

d:\src\learn>rand
5
49
552
60

(of course, the numbers change)

So I'm not inclined to go looking for the "bug" quite yet. Did you do an
srand() at the start of your test program?
-leor

 

[Howard]

Dear programmers,

Have a look at this function to generate random numbers from 1 to count.
It simply does not work. Well, actually it compiles silently, but then
returns "1" (Compiler: gcc/++ 3.3.2-r5; Gentoo Linux).

#include <cstdlib> // For random number generator
#include <ctime> // For time function
using namespace std;

inline int random(int count) {
return 1 +
static_cast<int>((count*static_cast<long>(rand()))/(RAND_MAX+1));
}

I was able to find out that the command _will_ work as it is expected
with out RAND_MAX, but obviously doesn't make any sense, then.

Thanks in advance,
Moritz Beller

You're casting the result of that division to a long, but the division
itself results in a value of at least 0 but less than 1. So, casting it to
a long truncates it to 0, always! I think if you remove the cast to long,
you'll get a better result.

-Howard

"All programmers write perfect code.
....All other programmers write crap."

"I'm never wrong.
I thought I was wrong once,
but I was mistaken."

 

[Victor Bazarov]

Dear programmers,

Have a look at this function to generate random numbers from 1 to count.
It simply does not work. Well, actually it compiles silently, but then
returns "1" (Compiler: gcc/++ 3.3.2-r5; Gentoo Linux).

What do you expect it to return?

#include <cstdlib> // For random number generator
#include <ctime> // For time function
using namespace std;

inline int random(int count) {
return 1 +
static_cast<int>((count*static_cast<long>(rand()))/(RAND_MAX+1));
}

I was able to find out that the command _will_ work as it is expected
with out RAND_MAX, but obviously doesn't make any sense, then.

It is possible that in your implementation of 'rand' it always returns 0
if unseeded. Besides, you will always get the same sequence of pseudo-
random numbers. You need to use 'srand'.

V

 

[Moritz Beller]

What do you expect it to return?

Random numbers from 1 to count.

It is possible that in your implementation of 'rand' it always returns
0 if unseeded. Besides, you will always get the same sequence of
pseudo- random numbers. You need to use 'srand'.

Usage in context:

const int dimLimit = 100;
dimensions srand((unsigned)time(0));
[...]
int rnumber = random(dimLimit);

Moritz Beller

 

[Victor Bazarov]

Random numbers from 1 to count.

numberS ?

It is possible that in your implementation of 'rand' it always returns
0 if unseeded. Besides, you will always get the same sequence of
pseudo- random numbers. You need to use 'srand'.

Usage in context:

const int dimLimit = 100;
dimensions srand((unsigned)time(0));
[...]
int rnumber = random(dimLimit);

I am sorry, perhaps I am dumb or something, but how do you expect it
to return "numberS" (yes, plural form, as you used) if you only call
it _once_? Just a reality check.

V

 

[Howard]

inline int random(int count) {
return 1 +
static_cast<int>((count*static_cast<long>(rand()))/(RAND_MAX+1));
}

Is it possible that you got the parentheses wrong in your actual code, and
have grouped together the division inside the static_cast<long>? That would
result in 1 + (count * 0), or 1, as the result.

Or, is it possible the this implementation does the division first, and
treats the result of that division as a long (since both operands are
integral), thus resulting in 0 for the division? (Just a wild-assed guess.)
When I get confused about precendence and grouping, I tend to add more
parentheses, just to be sure. You could try adding parentheses around the
multiplication part to be positive that it's multiplying before dividing.

Alternatively, you could try casting the rand() result as a double instead
of a long. (BTW, why *are* you casting as a long there, when you'll be
casting as int at the end anyway? Just to be sure it's big enough?)

-Howard

"All programmers write perfect code.
....All other programmers write crap."

"I'm never wrong.
I thought I was wrong once,
but I was mistaken."

 

[Moritz Beller]

numberS ?

ouh, I'm so sorry, one number, of course (YALL -- Yet Another Lapsus
Linguae).

Moritz

 

[Pete Becker]

static_cast<int>((count*static_cast<long>(rand()))/(RAND_MAX+1));

rand() returns an integral type. RAND_MAX is an integral type. When you
divide two integral types the result is an integral type. Since RAND_MAX
+ 1 is always greater than the value returned by rand(), the result of
the division is always 0.

Change the 1 to 1.0 and remove the casts.

 

[Leor Zolman]

rand() returns an integral type. RAND_MAX is an integral type. When you
divide two integral types the result is an integral type. Since RAND_MAX
+ 1 is always greater than the value returned by rand(), the result of
the division is always 0.

Change the 1 to 1.0 and remove the casts.

static_cast<int>
(
(
count * static_cast<long>(rand())
)
/
(RAND_MAX+1)
);

Just a public service ;-)
-leor

 

[Pete Becker]

static_cast<int>
(
(
count * static_cast<long>(rand())
)
/
(RAND_MAX+1)
);

Just a public service ;-)

Too many parentheses. :-( But I'll still bet that changing the 1 to 1.0
and removing the casts "fixes" the problem. The code assumes that
int*int will fit in a long, which ain't necessarily so. Since int and
long are the same size on most systems these days, the casts probably
don't matter. And if RAND_MAX is equal to INT_MAX (as it is with our C
library on 32-bit platforms, for example) then RAND_MAX+1 (which is of
type unsigned) is greater than any value that count*rand() will produce,
and the quotient will be 0.

 

[Leor Zolman]

Too many parentheses. :-( But I'll still bet that changing the 1 to 1.0
and removing the casts "fixes" the problem. The code assumes that
int*int will fit in a long, which ain't necessarily so. Since int and
long are the same size on most systems these days, the casts probably
don't matter. And if RAND_MAX is equal to INT_MAX (as it is with our C
library on 32-bit platforms, for example) then RAND_MAX+1 (which is of
type unsigned) is greater than any value that count*rand() will produce,
and the quotient will be 0.

Ah yes. RAND_MAX is 0x7FFF on the gcc platform I tested on here; perhaps
it is INT_MAX on the OP's. Now I agree with you. This makes either the
second or third time I've been bitten by this issue, so perhaps I'll
remember about it from now on...
Thanks,
-leor

 

[Victor Bazarov]

ouh, I'm so sorry, one number, of course (YALL -- Yet Another Lapsus
Linguae).

But then you get your number. If you call this function repeatedly,
you should get numbers that are different. The starting number will
depend on the seeding method you select and the ability of your
generator to be properly seeded.

I used your function with a small driver and it worked fine on VC++.
IIANM, Leor did too, and got good results.

Victor

Victor

 

[Pete Becker]

I used your function with a small driver and it worked fine on VC++.
IIANM, Leor did too, and got good results.

Yup. The problem is a subtle platform dependency. The code assumes that
count*(RAND_MAX+1) fits in a long. If RAND_MAX == LONG_MAX it won't
work.

 

[Moritz Beller]

rand() returns an integral type. RAND_MAX is an integral type. When
you divide two integral types the result is an integral type. Since
RAND_MAX+ 1 is always greater than the value returned by rand(), the
result of the division is always 0.

Resulting from this I concluded what we really need is a double, isn't
it? Here we go:

Change the 1 to 1.0 and remove the casts.

Which didn't make any differences at all. (Besides a mass of warnings by
the compiler.)

Moritz Beller

 

[Pete Becker]

Resulting from this I concluded what we really need is a double, isn't
it?

Here we go:

static_cast<int>((count*static_cast<double>(rand()))/(RAND_MAX+1.0));

Get rid of the casts and most of the parentheses. I can't parse this
without devoting more attention to it than I want to. <g> RAND_MAX + 1.0
is of type double, so the compiler promotes the result of rand() to
double when it evaluates rand()/(RAND_MAX+1.0). Similarly, count gets
promoted to double for the multiplication:

return 1 + count * (rand() / (RAND_MAX + 1.0));

Which didn't make any differences at all. (Besides a mass of warnings by
the compiler.)

Well, you shouldn't get a mass of warnings, so it sounds like there's
something else going on.

 

[Allan W]

You're casting the result of that division to a long, but the division
itself results in a value of at least 0 but less than 1. So, casting it to
a long truncates it to 0, always! I think if you remove the cast to long,
you'll get a better result.

That's not how I read it. It looks to me like the cast to long is happening
before the division, even.
Here's the same expression with newlines added:

1 +
static_cast<int>(
(
count*static_cast<long>(rand())
)/(RAND_MAX+1));

Change the parens like this:
1+static_cast<int>( count * (rand() / (RAND_MAX + 1.0)) )

 

[Moritz Beller]

Yes. That's what happens when you change the 1 to 1.0 <g> (of course,
I meant the 1 after RAND_MAX).

Well, not in fact: return 1 +

Get rid of the casts and most of the parentheses. I can't parse this
without devoting more attention to it than I want to. <g> RAND_MAX +
1.0 is of type double, so the compiler promotes the result of rand()
to double when it evaluates rand()/(RAND_MAX+1.0). Similarly, count
gets promoted to double for the multiplication:

return 1 + count * (rand() / (RAND_MAX + 1.0));
(...)

Well, you shouldn't get a mass of warnings, so it sounds like there's
something else going on.

Here is the output I get, just to convince you:

ex14_1.cpp: In function `int random(int)':
ex14_1.cpp:13: warning: return to `int' from `double'
ex14_1.cpp:13: warning: argument to `int' from `double'

Moritz

 

[Moritz Beller]

On 3 Jun 2004 17:57:37 -0700

That's not how I read it. It looks to me like the cast to long is
happening before the division, even.

Yup, right.

Change the parens like this:
1+static_cast<int>( count * (rand() / (RAND_MAX + 1.0)) )

Now we come to logic: I multiplied the result of rand() with count and
then divided it through (RAND_MAX + 1.0) [see 1], whereas you first
divide rand() through (RAND_MAX + 1.0) and then make a multiple of it
with count [2].

Excerpts:
[1](count*static_cast<double>( rand() )) / (RAND_MAX+1.0)

[2]( count * (rand() / (RAND_MAX + 1.0)) )

Moritz

 

[Pete Becker]

Here is the output I get, just to convince you:

ex14_1.cpp: In function `int random(int)':
ex14_1.cpp:13: warning: return to `int' from `double'
ex14_1.cpp:13: warning: argument to `int' from `double'

I wouldn't have called that a "mass" of warnings. You're getting two
warnings for the same thing. It's legal and well defined (in this case).
Put in a cast if you must, or shut of the warning.