SciRuby / nmatrix

Dense and sparse linear algebra library for Ruby via SciRuby
Other
469 stars 133 forks source link

NMatrix#rand needs to accept an existing random number generator #566

Closed translunar closed 7 years ago

translunar commented 7 years ago

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.

https://github.com/SciRuby/nmatrix/blob/7f3fa5d2e1e858b5c7df7136fd899da7f590c018/lib/nmatrix/shortcuts.rb#L536

shardulparab97 commented 7 years ago

@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?

translunar commented 7 years ago

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

Shailesh-Tripathi commented 7 years ago

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?

translunar commented 7 years ago

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

Shailesh-Tripathi commented 7 years ago

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?

translunar commented 7 years ago

...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.

Shailesh-Tripathi commented 7 years ago

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

translunar commented 7 years ago

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

NakulVaidya commented 7 years ago

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

translunar commented 7 years ago

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