shogun-toolbox / shogun

Shōgun
http://shogun-toolbox.org
BSD 3-Clause "New" or "Revised" License
3.02k stars 1.04k forks source link

port parallel code using pthread #3605

Open vigsterkr opened 7 years ago

vigsterkr commented 7 years ago

There are several parts of the code that uses pthread for implementing parallel execution of the algorithms. These should be ported to use either OpenMP or C++11 std::thread (preferably openmp).

some examples UPDATED 2:

for finding the full list use: git grep HAVE_PTHREAD

piyushgoel997 commented 7 years ago

hi, I'm new to this and would like to take up this issue.

vigsterkr commented 7 years ago

@piyushgoel997 sure, PRs are welcome!

piyushgoel997 commented 7 years ago

Can you help me a bit in doing it since I'm also new to Open Source and Machine Learning? Like I found HAVE_PTHREAD at https://github.com/shogun-toolbox/shogun/blob/develop/src/shogun/classifier/svm/SVM.cpp#L22 then what do i need to do next?

vigsterkr commented 7 years ago

@piyushgoel997 in that case nothing. in the other case you can see that pthread is being used to run parallel code. you should convert that using OpenMP. all these are well documented libraries, so please read about them and then try to convert the code and send a PR. it's not a problem if it's not working 100%, but without an attempt on the code it's really hard to help.

piyushgoel997 commented 7 years ago

I made changes in Distance.cpp but I don't know if it's correct or not, so should I get it checked by you.

vigsterkr commented 7 years ago

@piyushgoel997 create a PR and it's gonna get tested and i can review it as well

piyushgoel997 commented 7 years ago

Made the pull request. Kindly review it. https://github.com/shogun-toolbox/shogun/pull/3607 Thanks.

micmn commented 7 years ago

I've started working on this issue to get familiar with shogun's codebase and I've made a PR, I'd like to receive some feedback if this is the right way to port code to OpenMP (removing helper methods and thread params structs).

3621

Also I think I've found a problem in CHashedWDFeaturesTransposed: in the else branch of dense_dot_range_helper sub_index is null. https://github.com/shogun-toolbox/shogun/blob/8fdf278dca78f98d4e06058a5ce3defd560d697d/src/shogun/features/hashed/HashedWDFeaturesTransposed.cpp#L471

vigsterkr commented 7 years ago

@micmn cool stuff. i've added there some comments. it very well be that there's a bug in the code fixes are more than appreciated.

OXPHOS commented 7 years ago

an easy one: https://github.com/shogun-toolbox/shogun/blob/develop/tests/unit/base/RefCount_unittest.cc#L11