I agree that "OMP_NUM_THREADS" and "omp_set_num_threads()" is enough to define how many threads are used.
The reason I like Shikhar's ExecutionPolicy template idea is that I think it will clean up the implementation. At the same time, it will help users avoid diving down to the nitty-gritty details of the framework we are using, calling OpenMP functions to select parts of the code that should not execute in parallel. Cleaner implementation: We can encapsulate the logic that decides how many threads will be used, to a single place in the mlpack code, which will be called from everywhere in the codebase. This will provide a common interface ( = template parameter) for all algorithms we want to parallelize, and will make it possible to change our logic or add more policies in one place to impact all of our algorithms. Hopefully this might also encourage other developers to parallelize their code (once they are done implementing, debugging, and profiling it). User-friendliness: We decide what the default behavior of an algorithm is (e.g, to use all threads) and a user will not need to provide any template parameters - the default case will be used. If the user wants only 1 thread to be used, they can simply pass a non-default value to the template. I would argue this is better and cleaner than making the user call OpenMP functions they possibly know nothing about. Code simplicity: Our demo might have been a bit hacky (using an enum directly in the OpenMP pragma). However, the implementation could change so that sequential_execution_policy and parallel_execution_policy are classes, both of which implement a numThreads() function. This way, the code would be something like int nThreads = EP.numThreads(); // mlpack code with our own logic #pragma omp parallel num_threads(nThreads) // for loop here This way, the openMP lines are really as few as possible. We avoid mixing thread calculation logic inside the actual algorithm, and the user does not even know we use OpenMP - only that we are parallel. On Tue, May 16, 2017 at 7:05 PM, Ankit Aggarwal < [email protected]> wrote: > Given that parallelism in mlpack is OpenMP in order to keep the codebase > as simple and maintainable as possible, I think that what you have > listed is not a concern. > > Sure. > > On Tue, May 16, 2017 at 11:30 PM, Ryan Curtin <[email protected]> wrote: > >> On Tue, May 16, 2017 at 11:27:05PM +0530, Ankit Aggarwal wrote: >> > I don't think just doing "omp_set_num_threads" will leads to optimal >> > execution. I don't think openMP is NUMA aware (I never dig that much >> deeper >> > in openMP). I think it maybe better to use simple std::thread along with >> > hwloc for NUMA awareness or pthread(they provide more control support). >> >> Given that parallelism in mlpack is OpenMP in order to keep the codebase >> as simple and maintainable as possible, I think that what you have >> listed is not a concern. >> >> -- >> Ryan Curtin | "Hey, tell me the truth... are we still in the >> [email protected] | game?" - The Chinese Waiter >> > > > _______________________________________________ > mlpack mailing list > [email protected] > http://knife.lugatgt.org/cgi-bin/mailman/listinfo/mlpack >
_______________________________________________ mlpack mailing list [email protected] http://knife.lugatgt.org/cgi-bin/mailman/listinfo/mlpack
