added logistic regression classifier

[aceveggie]

Logistic Regression is supervised learning technique. It can be used for digit recognition, action recognition, etc. A class for logistic regression has been added. A sample program, documentation and test for logistic regression has also been added.

[vkocheganov]

@mdim, can you review this PR, it seems to be fine with buildbot tests.

[aceveggie]

@vkocheganov That's weird. My PR didn't have warnings before, it had all greens. Do I have to do anything?

[vkocheganov]

Yeah this is indeed weird, will investigate what was the problem. Thanks @aceveggie.

@mdim , could you please take a look at it?

[aceveggie]

Fixed trailing white-spaces in above commit. Its weird that the http://pullrequest.opencv.org/ page showed all successes and all green. But, when I looked in "Doc" link, it showed a warning saying that there have been trailing white-space in my code. Anyways, I fixed it.

[aceveggie]

I wonder if anyone is reviewing this? @vkocheganov

[snosov1]

Sorry for the delay, @aceveggie. @kirill-kornyakov Kirill, please, have a look also.

[kirill-korniakov]

@mdim, do you think you will have a chance to review it?

[aceveggie]

@kirill-kornyakov @snosov1 Its been more than 3 weeks. I wonder if anyone actually looked at the code? Is there anything I could help to make it faster (like more documentation on the algorithm developed, etc.)? This algorithm is present in other ML libraries in Python(sklearn), Java (Weka), etc but missing in OpenCV. I've used this algorithm for my research work and classifying MNIST digits in Machine Learning class (on coursera.org).

[kirill-korniakov]

Sorry, but the issue is the same as here: https://github.com/Itseez/opencv/pull/1188#issuecomment-22943151

[mdim]

@aceveggie Sorry for the long response! I hope to find a time to review the PR by the end of the week. Now most of OpenCV team supports the lib in free time, so the progress is not as fast as we want. Also reviewing new functionality really takes much time (vs eg bugfix).

[aceveggie]

This is going to a while to fix. Thanks for the feedback :)

[aceveggie]

@mdim: I appreciate your patience on this. I will push the changes and get back to you. This is my first ever Pull Request on Github. I could've just written a sample program, but I wanted everyone to use Logistic Regression with OpenCV so wrote it as a part of API.

[mdim]

@aceveggie Agree, Logistic Regression will be useful in OpenCV. For the first PR you've done good job! you've provided all we usually ask people (sample, tests..), thank you! But lets try to make it a bit better. I've started, but not finished the review of your code, just have made some comments about API. Before I continue could you please consider the comments and my main suggestion to use cpp version of OpenCV API in LR implementation. I know it might require a lot of your efforts to convert the code from C to C++ API, but we try not to use outdated C API now. ML module is going to be refactored in future. It would be great if you refactor your classes yourself before pushing it in OpenCV. Thanks again! Then I'll review the algorithmic details.

[vpisarev]

Hello! the code should be put to some module (e.g. "ml_extra") to github.com/itseez/opencv_contrib

[aceveggie]

@vpisarev : Should I put this in opencv_contrib ("ml_extra")? It is a new API feature and not a sample program.

[aceveggie]

@mdim: Let me know what you think of my comments. I'm almost done(made all changes you recommended on my local repo). After your input on those certain comments, I will change it and push the changes.

[aceveggie]

@mdim: I know you are kind of busy. But, could you just give your feedback on the comments I made. I almost finished it up. I can push the changes to this repo from my local repo.

[mdim]

Done. Sorry for the late feedback!

[aceveggie]

@mdim : I made all the changes you asked me. I can't seem to fix this warning of "/home/user/slave/builds/precommit_docs/src/opencv/modules/ml/doc/logistic_regression.rst:114 error 003: invalid base class documentation"

[aceveggie]

@mdim: I pushed all the changes that you wanted me to make. Build status is also showing all success at http://pullrequest.opencv.org

[aceveggie]

@mdim : Let me know if you need something else.

[mdim]

@aceveggie The code is getting better. Thanks! Left some more issues in the comments :)

[BKNio]

@etalanin Evgeny, i can't check old PR on http://pullrequest.opencv.org/ for example this one. Could you please check?

[aceveggie]

I fixed the errors and I'm waiting for the build status to update. I updated it yesterday.

[aceveggie]

@mdim: made requested changes to API and appropriate changes in documentation too. Just checked build-bot status. It showed all successes! :+1:

[aceveggie]

Here is a comparative analysis with Naive Bayes classifier (for Logistic Regression). http://select.cs.cmu.edu/class/10701-F09/slides/logistic-regression-annotated.pdf

[vkocheganov]

@aceveggie , @mdim what a patience you have guys to deal with such a big PR :-) Masha, do you think you can take hopefully a final look at LR classifier?

P.s. .pdf presentation? @aceveggie , you are really fan of what you do :-) and it's really great! Sorry if I lost the clue of conversation, but what was the purpose of comparison?

[aceveggie]

@vkocheganov : LOL :). I love OpenCV! I learnt a great deal(internal workings) about OpenCV by doing this PR. Also, this is my first ever PR on Github (or anywhere else). So, I'm very passionate. I'm trying to patient here. @mdim has been very patient with my C++ skills here. I deal with multiple programming languages at a time. So, I tend to forget the best practices in a programming language(hence, the long conversations :)). The pdf presents a comparative analysis of LR classifier with Naive Bayes. I just don't want this PR to be shrugged off as something that is not necessary in OpenCV. This classifier will be a great addition to OpenCV.

[vkocheganov]

I see, all you are saying @aceveggie makes sense! Your PR is very useful and will definitely not be shrugged off, and you likely see @mdim 's efforts on it. But I should say it will take a while to get to master branch, since @mdim is very-very busy and nobody else will venture to take responsibility for Machine Learning module :-)

Keep it up, @aceveggie !

[AnnaPetrovicheva]

@mdim Sending ping to you:) This contribution could be a marvelous New Year present for OpenCV!

[aceveggie]

I would love to see some progress on this!

[kirill-korniakov]

@mdim, should we take it? Or there is something else you want to see improved?

[aceveggie]

fixed other type conversion warning. Now build status is showing all success.

[BKNio]

@mdim all green, your move

[aceveggie]

@mdim do you want me to test this against existing Logistic Regression implementations anywhere and report recognition accuracy? I guess that would make your job easier.

[Daniil-Osokin]

There is failed OCL builder. Lets re-run builders.

[aceveggie]

This has been almost an year.

[PhilLab]

Nice contribution, it comes in really handy! However, http://pullrequest.opencv.org/ now only shows "not_queued" for all the builds - is there something wrong?

[vpisarev]

ML has been recently refactored; I agree that LR is a useful tool; I think, it should be refactored to match the style of new ml

[vpisarev]

@aceveggie, once again, thank you very much for the PR! And very, very sorry for delay; it has been another difficult period; finally we managed to merge it in.

[aceveggie]

@alalek: I can't seem to find the exact messages exchanged previously on why the existing licensing choice was used (Intel open source license). I can change it to the recommended license (if I can do that). Please let me know.

[alalek]

I have prepared PR with updated OpenCV license headers here: #19741 @aceveggie Could you please take a look and approve PR if there are no objections?