Crash when parallelizing predict

[skn123]

I have the following scenario: a.) I have trained a GClasses::GBag model with Gaussian Process and Naive Bayesian classifiers b.) I have successfully created the model c.) I use this code snippet to run the prediction

void MyFunc(GClasses::GSupervisedLearner* gModel,....)

ifdef USE_OPENMP

pragma omp parallel for

endif

for (long i = 0; i < gFeatures->rows(); ++i) { //predict the label for input features gModel->predict(gFeatures->row(i), gLabels->row(i)); }

If I disable the OpenMP macro, the code works fine (albeit slowly). However, when I enable OpenMP, the code crashes. So, the question is, can the prediction pipeline be made parallelizable via OpenMP (or in other words, is prediction thread safe)?

[mikegashler]

GNaiveBayes does not alter any state during predict, so it should be completely thread-safe. GGaussianProcess does store some intermediate values in member variables. However, I would only expect this to cause an incorrect prediction, not to cause a crash, so I would be interested in knowing more details about the crash.

You can make the predict method thread-safe for any of my models by making a deep copy of the model for each thread. One way to do this is to serialize and deserialize it. Now that you mention it, I see that I should probably provide a more convenient way to do that.

[skn123]

Mike, If you observe this snippet, it would be obvious that at this time the function is oblivious to the type of algorithms used to generate models. So, it would be nice to provide some documentation that indicates which models can be made thread safe and those which can't. Yes, your solution of making a deep copy of the model for each thread would work but that also hogs up memory! Now, imagine a situation where we have a bag of buckets :) or a bucket of bags ;) I think I will have to end up doing that and cannot use OpenMP for doing this. Let me dig a little bit deeper into this and figure out where the crash is being triggered from.

[mikegashler]

I see. Perhaps, a short-term solution would be to add an abstract method to the GSupervisedLearner class that would force every instance to declare whether or not its predict method is stateless. For example

  virtual bool isPredictStateless() = 0;

Then, one could programmatically determine whether it is necessary to deep-copy the model. (Fortunately, I think most of the really big models, such as GDecisionTree, are already stateless.) Then, the long-term solution would be to consider all of the cases where this method returns "true" to be a bug, and fix all my models to make stateless predictions. (In some cases, this will require some big design adjustments.)

Does this sound like the right solution to you?

[RadixSeven]

The abstract method seems like a good first step. However, it seems to me that for online algorithms, a stateless predict method is impossible. So, considering stateful prediction a bug would be a mistake.

On Friday, November 27, 2015 10:09 AM, mikegashler <notifications@github.com> wrote:

I see. Perhaps, a short-term solution would be to add an abstract method to the GSupervisedLearner class that would force every instance to declare whether or not its predict method is stateless. For example virtual bool isPredictStateless() = 0; Then, one could programmatically determine whether it is necessary to deep-copy the model. (Fortunately, I think most of the really big models, such as GDecisionTree, are already stateless.) Then, the long-term solution would be to consider all of the cases where this method returns "true" to be a bug, and fix all my models to make stateless predictions. (In some cases, this will require some big design adjustments.)Does this sound like the right solution to you?— Reply to this email directly or view it on GitHub.

[mikegashler]

I think "stateless" was not the right term for me to use, here. I think "reentrant" better expresses what I meant. The predict method certainly must depend on the current state of the model, but it would be nice if the predict method could also be called multiple times in parallel. This just requires storing any intermediate values for computing the prediction on the stack instead of in a common buffer associated with the model instance.

[skn123]

Mike, I like the new API design that you have for predict. The code snippet that I presented was from your codebase in sourceforge. I think the API should be as simple as possible. It would suffice to add an OpenMP pragma for each predict module for each model. So, algorithms that can be easily parallelized would benefit from it. Otherwise it can default to a serial implementation.

[skn123]

Here is where the code crashes; I am using a single RF classifier to classify my data.

[mikegashler]

Aha, I see the problem! A few years ago, I added a feature to enable make my ensembles to spawn multiple threads and make predictions in parallel. That feature is now fighting with OpenMP. To make my ensembles friendly to OpenMP, I would probably need to rip out that feature.

If you want to use my threading instead of OpenMP, you just need to call GEnsemble.setWorkerThreads(/n/), where /n/ is the number of threads you want to use. (Note that GBag inherits this method from GEnsemble.) Unfortunately, I was under pressure to meet some other deadlines when I wrote that feature, so I never really spent much time tuning or testing it. It is certainly not as polished as OpenMP, and I am not even confident that it will yield any speed-up.

For the long-term, we really need someone who has time to think deeply about how Waffles should integrate ensembles with threading. I would love to do it, but it's not really my area of expertise, and for the next few years I have to spend all my time begging the federal government for money so I can have a shot at getting tenure =).

On 12/01/2015 12:30 AM, skn123 wrote:

Here is where the code crashes; I am using a single RF classifier to classify my data.

image https://cloud.githubusercontent.com/assets/4594354/11493849/e0888664-9822-11e5-9424-92aad414a72f.png

— Reply to this email directly or view it on GitHub https://github.com/mikegashler/waffles/issues/2#issuecomment-160869207.

[skn123]

Mike :) The great rat race for NSF funding... I was not able to do change the worker threads; so as a kludge I changed the constructor to 4 from 1 for the worker thread count and disabled OpenMP.

[skn123]

Is there some way to turn off threading? If I have a hook for that, then OpenMP may work. The code is very slow if I have a Random Forest with 100 trees !! I think that would be involve making GEnsemble to not use GThread at all. An overloaded function for that would be helpful

[mikegashler]

I think the problem is that GEnsemble::predict (in GClasses/GEnsemble.cpp) is not reentrant. It is not reentrant because: (1) It modifies the member variable m_pPredictMaster. (2) It indirectly calls GEnsemble::castVote, which modifies the member variable m_accumulator.

To make GEnsemble::predict reentrant, this method needs to be redesigned to avoid changing with the GEnsemble object. There is not currently a switch to do that.

If the GSupervisedLearner::predict method were marked "const", then the compiler would force us to make all of the predict methods reentrant. But that's a bigger job than I currently have time to perform.

[skn123]

Which leaves me with no choice but to make k-copies of the model and run each model copy in a single thread?

[RadixSeven]

Another alternative is to fix GEnsemble::predict adding a const overload and having the non-const version call the const version. Then when/if someone does get around to making GSupervisedLearner::predict const, they can just use your work. Thus you will have solved both your problem and the problems of several others (the person who fixes GSupervisedLearner::predict and anyone else who needs to use GEnsemble::predict in a multithreaded context).

On Monday, December 7, 2015 11:37 AM, skn123 <notifications@github.com> wrote:

Which leaves me with no choice but to make k-copies of the model and run each model copy in a single thread?— Reply to this email directly or view it on GitHub.

[skn123]

I have solved this problem by making copies of the model. So technically, you can close this thread. However, as you have suggested some enhancements in GEnsemble, you may want to tag this issue as a future enhancement.