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

[Medium]Consider using Mutex for RNG seed handling #191

Closed
Feynstein opened this issue May 8, 2020 · 5 comments
Closed

[Medium]Consider using Mutex for RNG seed handling #191

Feynstein opened this issue May 8, 2020 · 5 comments

Comments

@Feynstein
Copy link

Feynstein commented May 8, 2020

http://www.cplusplus.com/reference/mutex/mutex/ I'm not that used to using them so maybe someone that knows it more can say if it's the good object to use in that case and how to do it properly.

@Feynstein Feynstein changed the title Considering using Mutex for RNG seed handling Consider using Mutex for RNG seed handling May 8, 2020
@Feynstein Feynstein changed the title Consider using Mutex for RNG seed handling [CS-5][Medium]Consider using Mutex for RNG seed handling May 8, 2020
@Feynstein Feynstein changed the title [CS-5][Medium]Consider using Mutex for RNG seed handling [Medium]Consider using Mutex for RNG seed handling May 8, 2020
@AndrewBoyd
Copy link

What is the exact problem you are trying to solve? Wrapping a function in a mutex will stop multiple threads calling the function simultaneously - which may stop any static or global variables ending up in an invalid state.

However, if multiple threads call into this function, then there still may be some non-determinism in your rng calls. For instance, consider the function:

int getRand(){
 // Not actually that random
 static int x = 0;
 return x++;
}

and suppose you have two threads A and B, that both call this function.
If A calls getRand() first, and then B, then A will see the result 0, and B will see the result 1.
However, if B calls first, and then A, then the results will be reversed.

A mutex may prevent A and B calling getRand() at the same time, but will not remove any non-determinism.

@Feynstein
Copy link
Author

Feynstein commented May 8, 2020

I haven't had time yet to investigate the non-determinism issue. But from what Ive seen in issue #184 it seems like it's caused by the multi-threading. What I think happen, is that the seed gets reset somewhere during the run because of multiple instances of the random generator, or multiple calling and modification of a persistent object. It happens often in Monte Carlo codes if their object persistency is not consistent. It's just a ball park assumption I'm making here. So yeah its basically a thread safety issue. Also in issue #161

@weshinsley
Copy link
Collaborator

The non-determinism is a total red herring, don't waste any time on it.

All you need to know about it is in #161, where all potential data races are described by line number and what the issue is. You can run that part (which is first-time initialisation only) single threaded if you like, and it is deterministic. But the file generated with 1 thread is neither less, nor more correct than a realisation generated by the multi-threaded code. They are just slightly different dealings of people into their local workplaces.

@hipparchus2000
Copy link

It's a good point that weshinsley makes in the other thread about RNG per thread and a variable number of threads. I think the desire for determinism is to prove correctness of the implementation (a functional test post development or porting). This could be addressed by a test mode where number of threads is fixed to N for the test, and N constant seeds provided to the RNGs. Test data subset and expected results obviously provided for this mode. Similarly the race in the setup could be avoided for this test mode.

@weshinsley
Copy link
Collaborator

Closing this one - no benefit for Mutex around RNG calls (and great loss of performance), since the RNG is already thread-specific, thread-safe and race-free.

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

4 participants