views:

97

answers:

5

Hiya, my code currently has three functions each producing a massive array of random numbers. Im wondering if there is a way of just having one function returning a linked list or multidimensional array to make it a bit neater:

(copied from http://pastebin.com/Y5aE6XKS)

#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <time.h>
#ifndef RAND_MAX
#define RAND_MAX 2147483648
#endif
#define N 420000

double* rdm_X(void);
double* rdm_Y(void);
double* rdm_Z(void);

void main(void)
{
   double* Random_number_list_X = rdm_X();
   double* Random_number_list_Y = rdm_Y();
   double* Random_number_list_Z = rdm_Z();
   double X[N+1], Y[N+1], Z[N+1], density = 1, vol = 42.0;
   double sum = 0, sum_x = 0, sum_y = 0, sum_z = 0;
   int i;

   for (i = 0; i <= N; i++) {
      X[i] = 3 * Random_number_list_X[i] + 1;
      Y[i] = 7 * Random_number_list_Y[i] - 3;
      Z[i] = 2 * Random_number_list_Z[i] - 1;
      if ((Z[i]*Z[i]) + (sqrt(X[i]*X[i] + Y[i]*Y[i]) - 3)*(sqrt(X[i]*X[i] + Y[i]*Y[i]) - 3) <= 1) {
         sum += density;
         sum_x += X[i] * density;
         sum_y += Y[i] * density;
         sum_z += Z[i] * density;
      }
   }
   printf("(%.5lf, %.5lf, %.5lf)\n",
            sum_x/sum, sum_y/sum, sum_z/sum);
}

double* rdm_X(void)
{
   double* Random_number_list_X = calloc(N + 1, sizeof(double));
   int i;

   srand(time(NULL));
   for (i = 1; i <= N; i++) {
      Random_number_list_X[i] = (float) rand() / (float) RAND_MAX;
   }
   return Random_number_list_X;
}

double* rdm_Y(void)
{
   double* Random_number_list_Y = calloc(N + 1, sizeof(double));
   int i;
   sleep(1);
   srand(time(NULL));
   for (i = 1; i <= N; i++) {
      Random_number_list_Y[i] = (float) rand() / (float) RAND_MAX;
   }
   return Random_number_list_Y;
}

double* rdm_Z(void)
{
   double* Random_number_list_Z = calloc(N + 1, sizeof(double));
   int i;
   sleep(2);
   srand(time(NULL));
   for (i = 1; i <= N; i++) {
      Random_number_list_Z[i] = (float) rand() / (float) RAND_MAX;
   }
   return Random_number_list_Z;
}
+1  A: 

The only difference between your three functions is how they call sleep(). Surely you can collapse all three into a single function, and call it three times in a loop?

Oli Charlesworth
It generates the same list of random numbers if I do that
Jack Medley
Then don't reset your random seed each time.
Oli Charlesworth
+1  A: 

Only call srand() once per program invocation, typically inside main().

int main(void) {
    /* initializations */
    srand(time(NULL));

    /* rest of program, with no more calls to srand() */
}
pmg
+6  A: 

A few points:

  1. Don't define RAND_MAX yourself.
  2. main returns an int.
  3. Only call srand once.
  4. Eliminate the extra calls to srand, and use one function to initialize your arrays.
  5. You defined X, Y, and Z as arrays, but really only used/needed one value each.
  6. There seems to be no reason to use dynamic allocation since your arrays sizes are fixed.
Jerry Coffin
+1  A: 

Other people already addressed some issues with your program, but do you realize that you are leaking over 10 megabytes of memory every time you run it? free()...

Jonathan
How so? Im not entirely C competent yet (but I realise a memory leak is bad news!) so any info would be great
Jack Medley
@Jack: every `malloc()` (`calloc()` in your case) **needs** a corresponding `free()` or you have a memory leak. `N * 3 * sizeof (double)` > 10Mbytes. You should `free(Random_number_list_X)` at the end of main.
pmg
+4  A: 

I'm not the first to point out that you should only call srand once, but I'll explain why:

The more often you call srand the less random the output of rand is.

The rand function is a pseudo-random number generator. That means that it generates numbers that look random, and have mathematical properties corresponding to randomness, but they aren't actually random. The output of rand is actually a fixed, completely deterministic sequence of numbers.

Or, rather, it produces one of a large family of completely deterministic sequences. You select which of these sequences you want by providing a "seed" value using srand. When you give srand a seed x, the next output of rand will be the first number of the pseudo-random (but completely deterministic!) sequence identified by the seed x. In other words:

int foo(int x)
{
   srand(x);
   return rand();
}

Will return different values for different inputs, but for a given x, will always return the same value. Not at all random!

This is actually a useful feature, since if you discover a bug in a program that relies on the output of rand, you can reliably reproduce the bug by providing the same seed to srand in order to obtain the same sequence from rand and so the same behavior from your program.

The reason that you need to call srand once is because otherwise your program will always receive the same sequence of numbers from rand (the sequence identified by seed 1). The reason you do not want to call srand more than once (in most cases) is because you are then repeatedly forcing rand back to the beginnings of its sequences rather than letting it give you one of them in its entirety. While any given sequence has properties of randomness, the sequence of sequence-beginnings does not necessarily have this property.

Obviously, it's especially bad if you call srand repeatedly with the same seed, because then you're forcing rand back to the beginning of the same sequence every time, and so rand will always produce the same value -- exactly what you don't want.

The reason you commonly see srand(time(NULL)) is because the time is likely to be different between any two invocations of a given program, which means that every time the program runs it will use a different pseudorandom sequence. But time only returns the time to the granularity of seconds, so if you do this repeatedly within a single invocation of a program, as in yours, and less than one second elapses between calls to srand, you will be repeatedly re-seeding with the same seed, with ridiculous results, as you have observed.

Bottom Line: Call srand exactly once, before your first use of rand. Trust that the implementers of the C library wrote a decent pseudo-random number generator, and don't try to "increase randomness" by attempting to compensate for problems that don't exist.

Tyler McHenry
Actually the "default seed" is 1.
pmg
@pmg Right you are. Fixed.
Tyler McHenry