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

Alternative fix for issue #393 #401

Merged
merged 1 commit into from
Aug 26, 2015
Merged

Alternative fix for issue #393 #401

merged 1 commit into from
Aug 26, 2015

Conversation

mreineck
Copy link
Contributor

These changes remove the global mt_state variable. The RNG state is created whenever needed and passed to all functions that generate random numbers. This solves the multi-threading issue.
Related to this, I also reworked the cmontecarlo tests so that they need no longer be executed in order. Each test now creates and tears down its own environment and should no longer leak resources.

@mreineck
Copy link
Contributor Author

This PR is just a suggestion to judge the advantages and drawbacks of different solutions a bit more easily. It is mutually exclusive with #395.

@wkerzendorf
Copy link
Member

@orbitfold can you see if this is good?

@orbitfold
Copy link
Contributor

I don't really like polluting procedure argument lists with another argument which frankly does not belong there. I don't know. I think either is fine.

@orbitfold
Copy link
Contributor

Let's just go with this, I'll merge unless anyone objects.

@wkerzendorf
Copy link
Member

if it's not a big deal , from my side is fine. I'm trying anyways to keep out of the C-part 😉

@wkerzendorf
Copy link
Member

@orbitfold merge away - when you are happy.

@mreineck
Copy link
Contributor Author

Making it part of storage does not address the problem, because storage is not thread-private. We would still have to explicitly allocate enough instances of mt_state for the individual threads, and every thread would have to make sure it was accessing its "own" copy.
Yes, the interface is ugly. If we were writing C++, I could provide something much cleaner :)

orbitfold added a commit that referenced this pull request Aug 26, 2015
@orbitfold orbitfold merged commit cbcb1cf into tardis-sn:master Aug 26, 2015
@mreineck mreineck deleted the privatize_rng branch August 31, 2015 10:19
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

Successfully merging this pull request may close these issues.

3 participants