search

cnr-isti-vclab / piccante

The hottest High Dynamic Range (HDR) Library
http://vcg.isti.cnr.it/piccante/
Mozilla Public License 2.0
251 stars 60 forks source link

Feature request: Thread safe random number generation #5

Closed Era-Dorta closed 8 years ago

Era-Dorta commented 8 years ago

Results are not reproducible when using openMP.

Random number seeding is done with the rand() function for the samplers, like in the bilateral filter. When compiling with openMP, the function is called concurrently in each thread, but rand() is not thread safe.

banterle commented 8 years ago

it should be fixed now (at least it works on OS X), please have a go to it

Era-Dorta commented 8 years ago

Unfortunately it's still giving inconsistent results with the fix. However applying the same procedure in the point sampler random does work on my linux machine. Would it make sense to have the seed set to zero there as well? For the record, a there are more occurrences of rand() which might affect other filters.

banterle commented 8 years ago

Ah gosh, thanks for pointing this out. I will handle this "rand" issue from tomorrow, because I am travelling at the moment!

To your knowledge, is std::time thread safe? It could be a good alternative.

Era-Dorta commented 8 years ago

I'm sorry but I don't really know about thread safety for std::time. In the documentation for std::chrono::system_clock::now, which I've seen used as seed there are no warnings about thread safety. While for std::rand() it is specified clearly. However my use case is to get exactly the same image, I need reproducibility for the experiments. Maybe adding a macro to switch between a fixed seed and std::time could be a good solution.

banterle commented 8 years ago

I made some extra changes. Please have a look at them, and let me know what you think about those.

Era-Dorta commented 8 years ago

Works like a charm, thanks so much

banterle commented 8 years ago

cool, I am happy about that!