Use process-global generators with fixed seeds#332
Use process-global generators with fixed seeds#332JamesMcClung merged 1 commit intopsc-code:mainfrom
Conversation
germasch
left a comment
There was a problem hiding this comment.
As we talked about, I agree this is a good way to go.
I guess I don't understand why you used a shared_ptr, though. The way the implementation currently works I guess there's a singleton-like generator per process, so anything using it doesn't have to worry about it going away?
Well, maybe it's possible that the singleton one gets destructed first as the process finishes, and then something else that has a pointer to it uses it in its own destructor that ends up running later? Is that the thinking?
f353285 to
fe0294f
Compare
|
I did this a long time ago and don't remember my reasoning for using shared_ptr, but I was probably just trying to avoid a raw pointer on principle. Would a raw pointer, or even just the non-pointer object, be preferable in this case? |
fe0294f to
2a6269c
Compare
|
I agree with avoid raw pointers on principle. And a unique_ptr would clearly be in appropriate here. I googled a little bit on C++ and "singleton" (I think this pretty much is what this is), and it looks like usually people return a reference. I think that could be a suitable alternative, but it can be inconvenient in that reference member vars aren't assignable, so one needs to make sure to set them in the constructor. Of course, an alternative is to not even keep the reference around. I guess I have some slight preference to follow the singleton pattern (though I might be missing something in how this doesn't really apply to this situation). But I don't really care -- a shared_ptr is pretty explicit in that it reminds one that the rng is a shared resource, even though no reference counting is actually needed... |
|
I agree that the current implementation is basically the singleton pattern. I could copy this implementation, although this discussion has convinced me that the best solution is dependency injection, since tests should have more control over the rng than a singleton (or really any global) could provide. Dependency injection would, of course, necessitate adding a template parameter for every class that needs rng (unless there's a better way I haven't thought of). That's by no means a foreign concept to PSC 😛, so if you agree, I wouldn't mind taking a stab at it. Also, I guess there would still have to be a singleton rng, which would be the default template parameter. |
I don't know if this is desirable, but this makes runs more deterministic. It was definitely useful for testing/debugging some BGK stuff.
Not all random number generation in the code uses these generators (see #317 ), and even if it did, there could in principle be RNG dependent on other processes and thus sensitive to race conditions.