implemented predict_proba for OneVsRestClassifier #1416

AWinterman · 2012-11-26T21:59:52Z

This also involved writing function predict_proba_ovr to mimic the
methodology of existing code.

Note that in the multilabel case, the marginal probability of the sample having the given label is returned. These probabilities do not sum to unity, since the set of such probabilities over all labels do not partition the sample space.

This also involved writing function `predict_proba_ovr` to mimic the methodology of existing code.

amueller · 2012-11-26T22:01:33Z

ping @mblondel (who has probably seen it already any way)

AWinterman · 2012-11-26T22:54:24Z

Am I reading the travis traceback wrong? Because it looks as though it failed from something unrelated to my changes.

amueller · 2012-11-26T22:56:54Z

I was just looking at it ;)
Actually, I have this problem a lot. It seems that mldata.org doesn't like travis or something and downloading data from there randomly gives server errors.
The travis is just 2 days old.... With your next commit it should hopefully work.

mblondel · 2012-11-27T01:54:39Z

sklearn/multiclass.py

+
+    if not multilabel:
+        #then probabilities should be normalized to 1.
+        Y /= np.sum(Y,1)[:,np.newaxis].repeat(Y.shape[1],1 )


Why do you need repeat?

Since normalization just divided the probabilities by a constant, I wonder if we can normalize even in the multilabel case?

I used repeat to make the row sums into a matrix of the same shape as Y. I'm still unfamiliar with all of the available numpy methods, so if there's a better way I'm all ears.

I don't think we should normalize in the multilabel case, since these probabilities should not sum to one. I think these should be the marginal probability that the given sample has the label in question. It is entirely consistent that two labels both have a 90% probability of applying to a given sample.

There is "broadcasting" along axis. You don't need to repeat. Y /= np.sum(Y,1) should work I think.
+1 for not normalizing

It is entirely consistent that two labels both have a 90% probability of applying to a given sample.

Yes, makes perfect sense!

Y /= np.sum(Y, axis=1) is better for readability :)

mblondel · 2012-11-27T01:55:31Z

Can you add tests to sklearn/tests/test_multiclass.py? Thanks!

AWinterman · 2012-11-27T17:17:57Z

@mblondel I was going to start by testing that probabilities add to one, that the prediction is the argmax and that errors are raised on malformed input. Are there any other tests I should write?

mblondel · 2012-11-27T17:32:24Z

@AWinterman Sounds good. You also need to check the multilabel case.

amueller · 2012-11-27T20:23:06Z

sklearn/multiclass.py

@@ -12,7 +12,7 @@
 use these estimators to turn a binary classifier or a regressor into a
 multiclass classifier. It is also possible to use these estimators with
 multiclass estimators in the hope that their accuracy or runtime performance
-improves.
+improves. It is also possible to do multilabel classification.


A note should definitely be added somewhere around here. Maybe even more prominent. (also the formulation is a bit awkward at the moment).

amueller · 2012-11-27T20:30:33Z

Do we want "predict_proba" for estimators that only have decision function? In the multi-class case, if we renormalize, the "probabilities" are not calibrated anyway, right?

AWinterman · 2012-11-28T00:22:59Z

I wasn't sure how to go about implementing predict_proba for decision function only estimators, so I figured I'd hold off for wiser heads. I'd be glad to add that to my todo list if somebody can point me to some resources on how to do it.

I'm not sure what you mean by that second sentence. Is that an argument for implementing predict_proba for estimators which only have a decision function?

If anyone has any suggestions for additional testing methods, it would be much appreciated. Currently implemented: - Do probabilities sum to one in single label case? - Is a ValueError raised for base classifier with no predict_proba method? - Do you arrive at the same predictions from predict_proba and predict?

mblondel · 2012-11-28T03:53:07Z

Estimators that don't implement predict_proba will raise an AttributeError. This is the expected behavior in my opinion (can you add an assert_raises test fort that?). Calibration is currently being implemented in PR #1176.

Regarding the motivation for normalizing multiclass probabilities, you can cite "Transforming Classiﬁer Scores into Accurate Multiclass Probability Estimates" (KDD 2002).

amueller · 2012-11-28T09:03:40Z

@AWinterman Yes, it was supposed to be an argument.
@mblondel Ok, I'll have to look into your reference. My point was that I didn't see a reason not to implement predict_proba from decision function, as it seems to be as valid as what happens now - maybe your reference will convince me otherwise.

On the other hand, I realized that we don't implement predict_proba just because it is possible, so maybe leave it out.

mblondel · 2012-11-28T09:13:44Z

@amueller But decision_function can return negative values, unlike
predict_proba. So I don't think it is wise to use its output without
calibration and call it a probability.

amueller · 2012-11-28T09:18:42Z

(I would have used softmax but let's forget about it)

mblondel · 2012-11-28T14:53:48Z

sklearn/tests/test_multiclass.py

+    n_samples = 100
+    n_classes = 5
+    for multilabel in (False, True):
+        for au in (False, True):


Can you implement the tests for the multiclass and multilabel cases in two different functions? au is not used in the multiclass case.

AWinterman · 2012-11-28T16:05:38Z

Sounds like I should just remove the _check_has_proba function, and let the people working on calibration reveal a predict_proba method if it's appropriate.

It should just raise an attr. error if it shows

… improvements.

`test_ovr_single_label_predict_proba` wasn't checking consitency between `predict_proba` and `predict` correctly. Now it is. Nose tests pass except for 1 conerning PIL.

mblondel · 2012-11-30T07:23:22Z

sklearn/tests/test_multiclass.py

+    #predict assigns a label if the probability that the
+    #sample has the label is greater than than 0.5.
+    pred = np.array([l.argmax() for l in Y_proba])
+    assert_true(not (pred-Y_pred).any())


You can use assert_false for that :) Can you spellcheck the comments? There are many typos. Then run pep8 and we are good to merge.

amueller · 2012-12-01T18:13:28Z

sklearn/multiclass.py

+        labels both have a 90% probability of applying to a given sample.
+
+        In the single label multiclass case, the rows of the returned matrix
+        should sum to unity.


I'd say "one" but I guess "unity" is ok.

amueller · 2012-12-01T18:17:10Z

LGTM apart from the sphinx thing.

amueller · 2012-12-02T09:32:58Z

Thanks, merged (by rebase).

AWinterman added 2 commits November 26, 2012 13:25

implemented predict_proba for OneVsRestClassifier

f008ef8

This also involved writing function `predict_proba_ovr` to mimic the methodology of existing code.

forgot an except clause

89413ea

amueller mentioned this pull request Nov 26, 2012

mldata.org doesn't like Travis #1417

Closed

mblondel reviewed Nov 27, 2012
View reviewed changes

removed unnecessary repeat

fa1db7b

amueller reviewed Nov 27, 2012
View reviewed changes

AWinterman added 2 commits November 27, 2012 16:33

corrected doc for predic_proba, also caught few errors.

db82265

mblondel reviewed Nov 28, 2012
View reviewed changes

AWinterman added 4 commits November 28, 2012 09:43

divided test for predict_proba into two functions

3480728

removed check for predict_proba method.

cf4dd8d

It should just raise an attr. error if it shows

[pep 257](http://www.python.org/dev/peps/pep-0257/) and and other doc…

7f2f45f

… improvements.

corrected bad test in test_multiclass

d9824ba

`test_ovr_single_label_predict_proba` wasn't checking consitency between `predict_proba` and `predict` correctly. Now it is. Nose tests pass except for 1 conerning PIL.

mblondel reviewed Nov 30, 2012
View reviewed changes

AWinterman added 3 commits November 30, 2012 10:19

Flake8 Corrections made

6099703

spell checked

fb40af5

Spelling is checked, passes Flake8 without errors.

e43b4b1

amueller reviewed Dec 1, 2012
View reviewed changes

added backtick around self.classes_ in multiclass.py

59802d0

amueller closed this Dec 2, 2012

Uh oh!

implemented predict_proba for OneVsRestClassifier #1416

implemented predict_proba for OneVsRestClassifier #1416

Uh oh!

Conversation

AWinterman commented Nov 26, 2012

Uh oh!

amueller commented Nov 26, 2012

Uh oh!

AWinterman commented Nov 26, 2012

Uh oh!

amueller commented Nov 26, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mblondel commented Nov 27, 2012

Uh oh!

AWinterman commented Nov 27, 2012

Uh oh!

mblondel commented Nov 27, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amueller commented Nov 27, 2012

Uh oh!

AWinterman commented Nov 28, 2012

Uh oh!

mblondel commented Nov 28, 2012

Uh oh!

amueller commented Nov 28, 2012

Uh oh!

mblondel commented Nov 28, 2012

Uh oh!

amueller commented Nov 28, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AWinterman commented Nov 28, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amueller commented Dec 1, 2012

Uh oh!

amueller commented Dec 2, 2012

Uh oh!

Uh oh!