Random::ISAAC pseudo random number generator

[ysbaddaden]

As suggested by @Scidom in #329 here comes a Crystal implementation of the ISAAC generator.

I implemented the readable algorithm, since the fast C implementation uses lots of pointers, that don't make much sense in Crystal (I think). I'm not sure how much safer, or less biased, it is compared to MT19937. It's current performance is ~36% slower than MT19937: 0.072s vs 0.046s for 10 million numbers.

Notes:

• This patch changes the signature of Random.new/1 which now expects an engine class instead of a seed.
• I removed the srand methods that were broken —they called the missing Random#init_by_array method, which could be made to point to the engine's method (private for now). I'm not sure if they are required.

[ysbaddaden]

I'm wondering about Random.new_seed: shouldn't it use SecureRandom.random_bytes instead of Intrinsics.read_cycle_counter to seed the generators?

[jhass]

How about adding another overload def initialize(seed : Int, engine_class=Mt19937)?

[asterite]

Wow! You are amazing!

Seriously! One day @Scidom suggests we add an ISAAC generator, the other day you send a pull request! I really can't tell you how much I appreciate your (all of you) contribution.

The speed is OK for now, we can always optimize it later, specially because we have those accurate specs.

I'm thinking, maybe instead of having Random have an engine, why not make Random a module that can be included in a random generator implementation? What I don't like about the @engine variable is:

1. There's an indirection, so generating numbers through Random will be a bit slower than just using the engine (unless we turn Random into a struct)
2. If you create many Random instances with different engines, @engine will become a union type and then each call to it will result in a dispatch (unless we make Random be generic, Random(E), where E is the engine type).

But those "unless" solutions seem too complex. Instead, we can do this:

# Include this into your random engine. You only need to implement next_int
module Random
  def next_float
    next_int * (1.0/4294967295.0)
  end

  def next_int(max : Int)
      if x > 0
        next_int % x
      else
        raise ArgumentError.new "incorrect rand value: #{x}"
      end
  end

  # and so on...
end

Java almost does this: it has a Random class which by default uses this algorithm, and subclasses must override a next(int bits) method to provide other algorithms. Then there's, for example SecureRandom, which inherits Random.

We could do the same as Java, but I think it's cleaner if we use a module so that each Random implementation has its own internal state. Then we can make SecureRandom include Random to share some implementation, as suggested by @rhysd.

Then we can have a Random::DEFAULT constant that has an instance of Random::MT19937 (or whatever engine we consider good as a default), and have Random#rand and the top-level rand delegate to this default engine, maybe guarded by a mutex so all of these are thread-safe by default. We can also have Random::new just create an instance of a default implementation we choose:

puts Random.new #=> #<Random::MT19937:0x106503000>

I don't think it matters much that Random.new doesn't return an instance of a class named Random as long as it quacks like a random engine.

What do you think?

Oh, @ysbaddaden, also thanks for moving the engines to their particular source files, I was going to do that but you were faster! :-)

I'll accept your pull request, which is perfect, and then we can do those changes later.

[ysbaddaden]

Oh, I didn't expect the pull request to be merged so quickly!

I like your Random module suggestion @asterite. I'll do the changes. Better design, better performance, less indirections: that's a win!

[papamarkou]

Great to see that you implemented Isaac @ysbaddaden. A few general remarks in relation to the above thread:

1. Perhaps RNG is a more suitable name rather than the less descriptive Random for the base class or module. All these random devices are random number generators (RNG), which is a much more concrete concept than Random. Besides, the abbreviation RNG is an established one.

2. We shouldn't be thinking about unions of random devices. By that I mean that there must be only one random number generator at a time regulating the generation of random numbers. It is simply conceptually wrong to allow coexisting RNGs and we should avoid it by all means.

3. A Random module or class may encapsulate behaviour that is more generic and independent of RNG, thus being used by any RNG. For example, the initialization of the seed, which may entail a couple of different methods, could be used by any RNG. According to my mindset, this dictates structuring the code by distinguishing between Random and RNG, the latter being a subset and possibly a member of the former, but this is only one way of organizing the problem.