Fix and improve multiple places about random number generation and shuffling

[netheril96]

• cluster_seedgen() now uses system entropy source /dev/urandom whenever possible.
• Fix Caffe::RNG::operator=(). The original version will result in two shared_ptr pointing to the same object with different reference counts and ultimately use-after-free or double-free problem.
• O(N) instead of O(N^2) algorithm for ImageDataLayer::ShuffleImages().
• convert_imageset will now properly seed the random number generator before shuffling, no longer producing the same shuffled result each run.

[bhack]

There is some activity for add Caffe support on windows. See #15.
Random_device is cross platfrom but it is available only in c++11
Edit:
see also this

[bhack]

Is Fisher–Yates algorithm covered by std::random_shuffle?

[netheril96]

@bhack:
std::random_shuffle internally uses rand() as the random number generator, whose randomness quality is horrible. Besides, the original call to std::random_shuffle is not preceded by srand(), resulting in the same order every time.

C++11 actually has a std::shuffle, but sadly we cannot use it.

[bhack]

std::random_shuffle doesn't receive the random number generator as last (optional) parameter?

[netheril96]

@bhack

See http://stackoverflow.com/questions/19219726/what-is-the-difference-between-shuffle-and-random-shuffle-c

You cannot pass in caffe::rng_t a.k.a boost::mt19937 as the third parameter to std::random_shuffle. Instead, you need to write a functor class, instantiate it, and then pass to std::random_shuffle, which involves probably more code than reimplementing Fisher–Yates algorithm.

[wendlerc]

So is there no more shuffling going on during training?

[netheril96]

@bhack
To achieve cross platform compatibility without c++11, one can use boost::random_device. But it involves new dependency so I'm not sure if it is a good idea.

The original code cannot compile under Windows either, since getpid() is a POSIX function.

[netheril96]

@Mezn

I'm not sure what you mean. This changes only the implementation of ShuffleImages(), not how many times it is called.

[wendlerc]

@netheril96 It was a general question, does caffe shuffle the dataset during training after each epoch or not? Maybe I posted my question onto the wrong issue sorry.

[netheril96]

@Mezn I don't know the answer to your question. I didn't exactly look at other parts of ImageDataLayer.

[shelhamer]

@Mezn off-topic for this thread, but no: Caffe does not shuffle between
epochs for you. You can do so manually, but it doesn't seem to make
much difference for ImageNet scale data. I believe Yangqing commented on
this in the past if you do an issue search for shuffling.

On Monday, August 4, 2014, netheril96 notifications@github.com wrote:

@Mezn https://github.com/mezN I don't know the answer to your question.
I didn't exactly look at other parts of ImageDataLayer.

—
Reply to this email directly or view it on GitHub
#848 (comment).

Evan Shelhamer

[jeffdonahue]

@Mezn and @shelhamer, the ImageDataLayer does do a shuffle after every epoch if the shuffle option is enabled (https://github.com/BVLC/caffe/blob/master/src/caffe/layers/image_data_layer.cpp#L126) -- this PR doesn't affect that.

@netheril96 thanks for fixing my hacky shuffle and everything else.

To me this looks good to merge other than possibly the /dev/urandom issue -- I think it's ok since we have these cross-platform compatibility issues throughout the code currently, and the fallback to the old cluster_seedgen seems good enough to me.

[wendlerc]

thanks!

[shelhamer]

Re: shuffle I misspoke. ImageDataLayer does reshuffle across epochs while
the DataLayer for leveldb/ml db does not.

For merge this looks good to me. Agreed with @jeffdonahue about platform
compatibility.

Thanks for reviewing since you last worked on RNG Jeff!

On Monday, August 4, 2014, Jeff Donahue notifications@github.com wrote:

@mexn and @shelhamer https://github.com/shelhamer, the ImageDataLayer
does do a shuffle after every epoch if the shuffle option is enabled (
https://github.com/BVLC/caffe/blob/master/src/caffe/layers/image_data_layer.cpp#L126)
-- this PR doesn't affect that.

@netheril96 https://github.com/netheril96 thanks for fixing my hacky
shuffle and everything else.

To me this looks good to merge other than possibly the /dev/urandom issue
-- I think it's ok since we have these cross-platform compatibility issues
throughout the code currently, and the fallback to the old cluster_seedgen
seems good enough to me.

—
Reply to this email directly or view it on GitHub
#848 (comment).