nok / sklearn-porter

Transpile trained scikit-learn estimators to C, Java, JavaScript and others.
BSD 3-Clause "New" or "Revised" License
1.28k stars 170 forks source link

SVC (kernel=linear) JS prediction logic #22

Closed anmolshkl closed 6 years ago

anmolshkl commented 6 years ago

Hi @nok, the libsvm implementation seems to be using subtraction while the sklearn-porter's JavaScript predict method is using addition in the same place. I'm guessing both are the same if the intercepts are having opposite sign, but, I'm not sure. Could you please shed some light on this?

nok commented 6 years ago

Hello @anmolshkl , do you get odd predictions between the original and transpiled estimator? Can you post the data and your params? Nevertheless there isn't any error in the regression tests of that classifier (e.g. log). But that doesn't mean, that there isn't an error.

anmolshkl commented 6 years ago

@nok thanks for the quick response. I'm getting the right predictions, and, the accuracy of the transpiled estimator is same as the scikit estimator (frankly, that's why I'm a little confused). To give you a little context about my confusion - I was trying to implement my own estimator for SVC and I tried following libsvm's code for prediction but I just couldn't get it right. Then, I looked into your estimator and found out that sklearn-porter's transpiled estimator uses decisions[d] = tmp + inters[d] while libsvm uses sum -= model->rho[p]. I replaced subtraction with an addition, and, my code started predicting values as expected.

I was just curious and wanted to know if you also encountered this problem while writing the algorithm for the estimator? Also, do you happen to have any reference material you used for writing the SVC estimator algorithm?

nok commented 6 years ago

Today I don't know how I figured it out in the past, but in general I add console outputs in the original scikit-learn classes and the generated classes. Then I compared each output and fix any differences on the fly. Finally I wrote regression tests to cover changes in the future.