SciRuby/nmatrix

NMatrix#rand needs to accept an existing random number generator

translunar opened this issue · 10 comments

Right now it appears the user is unable to give this method an existing random number generator, which is bad. We want to be able to control the seed, for example.

def random(shape, opts={})

@MohawkJohn I would like to help.
Could you please explain this problem further.
Could you please explain what do you mean by giving the method an existing random number generator
and Do we want to pass the seed as an optional the parameter along with the scale?

Sure.

Random number generators aren't really random. They're pseudo-random. You have to give them a seed. Some provide an option where they come up with their own seed, but that isn't cryptographically secure.

If you're running Monte Carlos, you may want to get the exact same combination of random numbers every time you run it. You'd do that by providing the same seed every time.

Go look at the Ruby docs for Random: https://ruby-doc.org/core-2.2.0/Random.html

Hi @MohawkJohn ,
If the issue is still open, I would like to help.
I am not getting exactly what is required. The "Random.new" function is not provided with any seed, hence it uses different seed value every time. So you want that it should have the same seed value every time?

That's incorrect. Random.new accepts a seed, but by default creates its own. See source:

https://ruby-doc.org/core-2.2.0/Random.html#method-c-new

Hi @MohawkJohn
Yes I read the documentation. In the code Random.new is not provided with any seed value so it creates a random seed of its own.
So you want that it should be provided with the same seed every time?

...no. I think you need to re-read my comment. :)

I'll try to clarify.

You can call Random.new in two ways:

Random.new(some_int_value) # provide a seed
Random.new() # don't provide a seed

If you call it the second way, it's equivalent to doing Random.new(Random.new_seed), where Random.new_seed generates a seed for you.

Right now, NMatrix only supports option 2. I want it to support both options.

Hi @MohawkJohn
Sorry for the confusion. I get your point now :)
I would like to take up the task.

This looks like it should work. How do you plan to write the spec?

It looks like this issue should be closed. If not what exactly is missing in the above pull request?I would like to help.

@NakulVaidya It's missing a spec. You're welcome to contribute one.