Add common tests for consistent decision_function behavior in binary case

[amueller]

I think there is no common test right now to check that the decision_function behavior is consistent across all estimators. We check that it is consistent with predict_proba and predict if the classes are [0, 1], but I don't think there is a test for [-1, 1] or arbitrary strings. I vaguely recall hard-coded cases for this, so I think adding a test would be really good.

[NarineK]

Do you still need help with this issue ? May I work on it, @amueller ?

[jnothman]

Sure you may.

[NarineK]

Would this be the best place to add the test case ?
https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/utils/estimator_checks.py#L1249
@amueller , @jnothman

[amueller]

that tests already tests something like this, but uses integers for classes.
You could check out https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/utils/estimator_checks.py#L1394 which is multi-class, and maybe add a binary case and also check the decision functions there?

[NarineK]

I see. Let me check that one

[NarineK]

As I was writing the tests, I've noticed that the output of decision_function for NuSVC and SVC is inconsistent with predict when decision_function_shape='ovo'. It works fine with 'orv'
Here is an example:

classifier = NuSVC(decision_function_shape=‘ovo’)
X, y = make_blobs(n_samples=30, random_state=0, cluster_std=0.1)
X, y = shuffle(X, y, random_state=7)
X = StandardScaler().fit_transform(X)
X -= X.min() - .1
X = pairwise_estimator_convert_X(X, clf)
y_names = np.array(["one", "two", "three"])[y]
y_ = y_names
set_random_state(classifier)
classifier.fit(X, y_)
y_pred = classifier.predict(X)

decision = classifier.decision_function(X)
decision_y = np.argmax(decision, axis=1).astype(int)
dec_func = classifier.classes_[decision_y]

output:

dec_func:

>>> array(['two', 'three', 'two', 'two', 'two', 'one', 'two', 'one', 'one',
       'three', 'two', 'one', 'one', 'three', 'two', 'one', 'one', 'one',
       'two', 'one', 'one', 'three', 'two', 'three', 'three', 'two',
       'three', 'one', 'one', 'one'],
      dtype='|S5')

y_pred:
>>> 
array(['three', 'one', 'three', 'three', 'three', 'one', 'three', 'one',
       'two', 'one', 'three', 'two', 'two', 'one', 'three', 'two', 'two',
       'two', 'three', 'two', 'two', 'one', 'three', 'one', 'one', 'three',
       'one', 'two', 'two', 'one'],
      dtype='|S5')

Are you aware of this ? @amueller @jnothman
Should I exclude it from the tests ?

[jnothman]

Hmmm... Could you open a new issue (if this is not already covered by an existing one) please?

…

On 28 December 2017 at 16:35, NarineK ***@***.***> wrote: As I was writing the tests I've noticed that the output of decision_function for NuSVC and SVC is inconsistent with predict when decision_function_shape='ovo'. It works fine with 'orv' Here is an example: classifier = NuSVC(decision_function_shape=‘ovo’) X, y = make_blobs(n_samples=30, random_state=0, cluster_std=0.1) X, y = shuffle(X, y, random_state=7) X = StandardScaler().fit_transform(X) X -= X.min() - .1 X = pairwise_estimator_convert_X(X, clf) y_names = np.array(["one", "two", "three"])[y] y_ = y_names set_random_state(classifier) classifier.fit(X, y_) y_pred = classifier.predict(X) decision = classifier.decision_function(X) decision_y = np.argmax(decision, axis=1).astype(int) dec_func = classifier.classes_[decision_y] output: dec_func: >>> array(['two', 'three', 'two', 'two', 'two', 'one', 'two', 'one', 'one', 'three', 'two', 'one', 'one', 'three', 'two', 'one', 'one', 'one', 'two', 'one', 'one', 'three', 'two', 'three', 'three', 'two', 'three', 'one', 'one', 'one'], dtype='|S5') y_pred: >>> array(['three', 'one', 'three', 'three', 'three', 'one', 'three', 'one', 'two', 'one', 'three', 'two', 'two', 'one', 'three', 'two', 'two', 'two', 'three', 'two', 'two', 'one', 'three', 'one', 'one', 'three', 'one', 'two', 'two', 'one'], dtype='|S5') — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#10175 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEz6zhpvlhoCpn2QJTKBOXo8Oja3_qhks5tEyiOgaJpZM4Qk7g8> .

[NarineK]

I didn't see exactly the same issue and created: #10388.
Should I skip SVC test cases or wait for the issue to be fixed ?

[jnothman]

ovo is irrelevant to the binary case

[NarineK]

I see, I'll do the checks only for binary then.

[NarineK]

@jnothman , @amueller have you had time to take a look into my pull request?

[jnothman]

Sorry, I'm not sure how this flew under my radar. Will try take a look soon.