Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

srand not implemented, fails silently #639

Closed
wmnnd opened this issue Oct 31, 2014 · 8 comments
Closed

srand not implemented, fails silently #639

wmnnd opened this issue Oct 31, 2014 · 8 comments

Comments

@wmnnd
Copy link
Contributor

wmnnd commented Oct 31, 2014

Random::srand seems to be not implemented, instead it fails silently.
I find this problematic because it creates inconsistent results with other Ruby implementations. I have tried this code sample in both MRI and JRuby and received the same results.

On a side note, the methods of Random can only be accessed as Object::method in Opal and not as Random::method.

srand 123
a = [rand(100), rand(100), rand(100)]

srand 123
b = [rand(100), rand(100), rand(100)]

puts "#{a} == #{b}: #{a==b}"
#Expected: [66, 92, 98] == [66, 92, 98]: true

P.S.:
I know that JavaScript's own pseudorandom number generator doesn't support seeding. Would there be any other way to fix this?

@mieko
Copy link
Contributor

mieko commented Oct 31, 2014

Considering it'd require a PRNG implementation anyway to allow for srand, it'd be awesome if it matched MRI's output given the same seed value. Kernel#rand and #srand are implemented in terms of an instance of Random (Random::DEFAULT) in MRI, which we don't have an implementation of in Opal.

Dealing with the performance tradeoff, there could exist a Random::ECMA that ignores the seed value and acts as the current Kernel#rand does.

Edit: Here's a Javascript port of the same Mersenne Twister implementation MRI is based on, mt19937ar: https://gist.github.com/banksean/300494

@wmnnd
Copy link
Contributor Author

wmnnd commented Oct 31, 2014

@mieko I was just gonna post the link to that Gist too, because I hadn't seen your edit yet ;-)
The genrand_res53() of this implementation is actually compatible with MRI's implementation of rand and returns the same pseudorandom values if the same seed is used (except for using rand with a parameter which is not supported here).

@mieko
Copy link
Contributor

mieko commented Oct 31, 2014

Ah, cool to know they're compatible, @wmnnd.

Here's a sketch of Random, if anyone wants to play with it. It has not been tested or even ran (I don't have the connectivity to get a full Opal stack set up this weekend). Otherwise I'll get back to it later.

@daveheitzman
Copy link
Contributor

I'm interested in implementing this.

@wmnnd
Copy link
Contributor Author

wmnnd commented Nov 24, 2014

I have not tried the implementation by @mieko and just used a simple wrapper for the Mersenne twister JavaScript implementation. But @mieko's code seems a good place to start.

@daveheitzman
Copy link
Contributor

I did use mieko's code in my implementation

@daveheitzman
Copy link
Contributor

to be clear, I used this js in my implementation: https://gist.github.com/banksean/300494
I didn't use Mieko's sketch.

@iliabylich
Copy link
Contributor

Fixed in #1540

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants