c++ random number generation not random

Question

I'm trying to perform a random shuffle of a vector using Visual Studio 2013 C++. The following is the code that I have

    static void shuffle(vector<int>& a){
    int N = a.size();

    unsigned long long seed = chrono::system_clock::now().time_since_epoch().count();
    default_random_engine generator(seed);

    for (int i = 0; i < N; i++){
        uniform_int_distribution<int> distribution(0,(N-1)-i);
        int r = i + distribution(generator);
        swap(a[i], a[r]);
    }
}

My problem is when I call this method multiple times in succession the shuffle is not random. What could be wrong with the code?

Any help would be much appreciated.

Typically you'd want to use `random_device` to seed the generators. — Bartek Banachewicz, May 27 '14 at 08:48
AlexSavAlexandrov - it repeatedly generated the same number. — user3213700, May 27 '14 at 09:04
If it generates the same sequence if called multiple times in succession, then it's possible that you were calling it several times within the "tick" period of the selected clock, meaning `now()` would return the same value on each call, and your random generator would be seeded with the same value each time. Of course, whether that happens depends on the clock used - I think you can use `period()` to query the tick length? — icabod, May 27 '14 at 09:13
@icabod: `period` gives you the units, there is nothing that tells you the tick frequency. — Mooing Duck, May 27 '14 at 18:11

score 1 · Answer 1 · answered May 27 '14 at 08:57

1

Uhm, I'm curious... why isn't the following sufficient for your needs:

static void shuffle(vector<int>& a)
{
    // There are better options for a seed here, but this is what you used
    // in your example and it's not horrible, so we'll stick with it.
    auto seed (std::chrono::system_clock::now().time_since_epoch().count());

    // Don't bother writing code to swap the elements. Just ask the standard
    // library to shuffle the vector for us.
    std::shuffle(std::begin(a), std::end(a), std::default_random_engine(seed));
}

answered May 27 '14 at 08:57

Nik Bougalis

10,495
1
21
37

2

It is sufficient but I wanted to know why that did not perform a random shuffle. – user3213700 May 27 '14 at 09:05
`random_device` is a far better seed – Mooing Duck May 27 '14 at 18:12
1

It's because "Any one who considers arithmetical methods of producing random digits is, of course, in a state of sin". – May 27 '14 at 18:12

score -1 · Answer 2 · edited May 23 '17 at 10:32

std::shuffle dosent remove duplicates, it just swaps the positions of the random numbers generated.

How can I efficiently select several unique random numbers from 1 to 50, excluding x?

You can home cook your own shuffle code otherwise:

#include <ctime>
#include <string>
#include <vector>
#include <iostream>
using namespace std;


void myShuffleWithNoRepeats( int  random_once_buf[] , int size=100)
{
      srand(time(0));

      for (int i=0;i<size;i++)  
      {
          // call made to rand( ) , stored in random_once_buf[ ]
          random_once_buf[i]=rand() % 100;

          //////////////////////////////////////////////////////////////////////////////////////
          // The line below generates unique random number only once                          //
          //                                                                                  //
          // the variable i is the random_once_buffer[i] buffer array index count,            //
          // j is the check for duplicates, j goes through the random_once_buffer[i] buffer   //
          // from 0 to i at every iteration scanning for duplicates, reversing one step  if one duplicate is found..                         //
          //////////////////////////////////////////////////////////////////////////////////////

          for(int j=0;j<i;j++)  if (random_once_buf[j] == random_once_buf[i]) i--; 


      }
       cout<<"   \n\n\n ";

}



int main(void)
{

      const int size=100 ;
      int  random_once_buffer[100] ;


      // Call made to function myShuffleWithNoRepeats( )
      myShuffleWithNoRepeats( random_once_buffer , size );

      // Loop to display the array random_once_buffer[ ]
      for ( int i=0;i<size;i++) cout<<""<<random_once_buffer[i]<<"\t";

      cout<<" \nPress any key to continue\n";
      cin.ignore();
      cin.get();

  return 0;
}

This code accomplishes something entirely different from the original code. The original code *shuffles an existing list* of integers. Your code *generates a new list* of random integers. — Rob Kennedy, May 27 '14 at 18:13

c++ random number generation not random

2 Answers2