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

randomVector is buggy #97

Closed
yongqli opened this issue Nov 27, 2014 · 12 comments
Closed

randomVector is buggy #97

yongqli opened this issue Nov 27, 2014 · 12 comments

Comments

@yongqli
Copy link

yongqli commented Nov 27, 2014

*Core> import Numeric.LinearAlgebra.HMatrix
*Core Numeric.LinearAlgebra.HMatrix> randomVector 0 Gaussian 3
fromList [1.5870180704420729,-1.7984362733226869,-1.3134535927556386]
*Core Numeric.LinearAlgebra.HMatrix> randomVector 0 Gaussian 3
fromList [-1.7984362733226869,-1.3134535927556386,-9.385551900167438e-2]

It seems that randomVector is not a deterministic function of the seed. It also seems that randomVector is not thread safe, I kept seeing it generate random NaNs in multithreaded code.

@yongqli
Copy link
Author

yongqli commented Nov 27, 2014

Looks like it uses srand() and rand()

See this http://stackoverflow.com/questions/6161322/using-stdlibs-rand-from-multiple-threads

@yongqli
Copy link
Author

yongqli commented Nov 27, 2014

random_vector_GSL isn't thread safe either, due to static gsl_rng * gen = NULL; One possibility would be to call gsl_rng_alloc and gsl_rng_free per call, although that might be slow.

@albertoruiz
Copy link
Collaborator

This is a big problem!

Perhaps we can fix randomVector using drand48 as suggested in SO. A temporary fix for random_vector_GSL would be also good even if it is slow.

Thanks for the bug report.

@albertoruiz
Copy link
Collaborator

I have changed rand() to drand48_r. Can you check that it works now as expected?

@yongqli
Copy link
Author

yongqli commented Dec 1, 2014

I haven't had a chance to test it, but wouldn't the use of static int phase = 0; still not be thread safe? At the very least it wouldn't be deterministic result of the seed, as it is keeping hidden state.

@albertoruiz
Copy link
Collaborator

Of course! I forgot that state. I will fix it.
Thanks!

@albertoruiz
Copy link
Collaborator

I have pushed a fix. I think I have removed all static variables.

@yongqli
Copy link
Author

yongqli commented Dec 2, 2014

I can't compile it on my machine but it looks good.

src/C/vector-aux.c:738:25:
     error: variable has incomplete type 'struct drand48_data'
        struct drand48_data buffer;

@yongqli yongqli closed this as completed Dec 2, 2014
@albertoruiz
Copy link
Collaborator

Do you know the reason for the error? I have gcc 4.6.3 and it compiles without any warning.

@yongqli
Copy link
Author

yongqli commented Dec 3, 2014

I'm on OSX, so my gcc is really clang:

% gcc --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 6.0 (clang-600.0.56) (based on LLVM 3.5svn)
Target: x86_64-apple-darwin14.0.0
Thread model: posix

@yongqli
Copy link
Author

yongqli commented Dec 3, 2014

I might also not have the build environment set up correctly.

@albertoruiz
Copy link
Collaborator

There is a new version using random_r(), I hope it works well on OSX.

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

No branches or pull requests

2 participants