MRG LibLinear class weights fix #1491
Conversation
|
The behavior us now consistent within sklearn (I am a bit puzzled by RidgeClassifier, see #1489) and also the same as for the LibSVM binary. Renamed to MRG. |
|
travis is not happy. |
|
@agramfort thanks, forgot that one :( |
|
Ok, I skipped that test for the moment. It's not really related to the bug the PR fixes. |
|
Still not working :( I can't generate a dataset with 3 classes where balancing helps the f1 score |
|
ok, should be good now. |
|
Can you summarize what was the problem? |
|
There used to be a problem in the class order differing in liblinear/libsvm and sklearn. I fixed that some time ago by switching the classes in the liblinear and libsvm code. This fixed the sign of the decision function and for some reason also the class weights in the SVC. The class weights in LinearSVC remained broken. It looks to me as if the class-weights were never correct in the binary case - and the tests really didn't test anything. |
| @@ -107,6 +107,9 @@ Changelog | |||
| Gaussian and sparse random projection matrix | |||
| by `Olivier Grisel`_ and `Arnaud Joly`_. | |||
|
|
|||
| - Fixed `class_weight` support in :class:`svm.LinearSVC` and | |||
| :class:`linear_model.LogisticRegression` by `Andreas Müller`_. | |||
There was a problem hiding this comment.
Could give some more details here?
There was a problem hiding this comment.
If I am not mistaken, SVC and NuSVC are also impacted. At least the dtype of the prediction outcome is now integer instead of float.
There was a problem hiding this comment.
The changes to SVC and NuSVC come from #1485, so I should add the comment there.
|
Besides the minor comments above, +1 for merging. |
|
@ogrisel I added the changes to the SVC |
|
+1 for merge |
|
Thanks for the reviews @mblondel :) |
Fixes #1411.
It seems I broke that in 5427f81 - or rather that I didn't fix there when I fixed it for libsvm.
The current tests for class weights seem suboptimal.
This branch contains tests for all estimators supporting class_weight.
I will also try to improve the tests in the svm module.If everything works, I'll add another comment to linear.cpp.
I want to check if the corrected behavior is consistent with the libsvm binary (see comments in #1411), but I feel being consistent inside sklearn is more important any way.This PR is on top of #1485 because they touch the same stuff.