[WIP] Sample weight consistency by ainafp · Pull Request #5515 · scikit-learn/scikit-learn

ainafp · 2015-10-21T15:03:28Z

Supersedes #5461

Added a 0/1 sample weight test. Failing estimators are

AdaBoostRegressor, BaggingClassifier, BaggingRegressor, CalibratedClassifierCV, LogisticRegressionCV, Perceptron, RandomForestClassifier, RandomForestRegressor, RidgeCV, RidgeClassifierCV, SGDClassifier, SGDRegressor

For the previous test,

AdaBoostRegressor, BaggingRegressor, DecisionTreeRegressor, ExtraTreesRegressor, GradientBoostingRegressor, LogisticRegressionCV, Perceptron, RandomForestRegressor, 
RidgeCV, RidgeClassifierCV, SGDClassifier, SGDRegressor

The difference between the 2 are:

BaggingClassifier, CalibratedClassifierCV, DecisionTreeRegressor, ExtraTreesRegressor, GradientBoostingRegressor, RandomForestClassifier

How do I proceed? Do I create an exclusion list or an inclusion list (p.ex. linear models) for the previous test?

ping @eickenberg, @amueller, @glouppe, @arjoly, @agramfort, @GaelVaroquaux

…ck whether test should apply to all of the estimators

…nd 1

eickenberg · 2015-10-21T15:09:17Z

sklearn/tests/test_common.py

@MechCoder what did you mean by the comment

a if name == check might be better.

that you put here?

I meant instead of try catch block, it would be safer to have a list of Estimators that have sample_weight in the fit param but might not support it. (which in this case is just LogisticRegression)

wdyt

amueller · 2015-10-21T15:57:11Z

see if you can make the SGDClassifier and Perceptron working maybe? might be a convergence issue. Or we ask to high a precision on the coef.

…for SGD estimators to change number of iterations or precision.

ainafp · 2015-10-22T09:38:45Z

I couldn't make SGDClassifier and Perceptron work, so I exclude them also. The excluded are
'AdaBoostRegressor', 'BaggingClassifier', 'BaggingRegressor', 'GradientBoostingRegressor', 'LogisticRegression', 'LogisticRegressionCV', 'LinearSVC', 'LinearSVC', 'MultinomialNB', 'CalibratedClassifierCV', 'SGDClassifier', 'SGDRegressor', 'Perceptron', 'RidgeClassifierCV', 'RidgeCV', 'RandomForestClassifier', 'RandomForestRegressor'
RidgeCV is being fixed in #4490
CalibratedClassifierCV is being discussed in #5518

Tests are now passing. Please take a look at the exclusion lists and see if you are OK with that

eickenberg · 2015-10-22T12:06:43Z

Problems with Travis:

  File "/home/travis/build/scikit-learn/scikit-learn/sklearn/tests/test_common.py", line 232, in test_get_params_invariance

    yield check_transformer_n_iter, name, estimator

NameError: global name 'estimator' is not defined

======================================================================

FAIL: sklearn.tests.test_common.test_sample_weight_consistency(array([1, 1, 0, 1, 1]), array([1, 1, 0, 1, 0]), 6, 'AdaBoostClassifier prediction not equal')

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/home/travis/build/scikit-learn/scikit-learn/testvenv/local/lib/python2.7/site-packages/nose/case.py", line 197, in runTest

    self.test(*self.arg)

  File "/usr/lib/python2.7/dist-packages/numpy/testing/utils.py", line 800, in assert_array_almost_equal

    header=('Arrays are not almost equal to %d decimals' % decimal))

  File "/usr/lib/python2.7/dist-packages/numpy/testing/utils.py", line 636, in assert_array_compare

    raise AssertionError(msg)

AssertionError: 

Arrays are not almost equal to 6 decimals

AdaBoostClassifier prediction not equal

(mismatch 20.0%)

 x: array([1, 1, 0, 1, 1])

 y: array([1, 1, 0, 1, 0])

>>  raise AssertionError('\nArrays are not almost equal to 6 decimals\nAdaBoostClassifier prediction not equal\n(mismatch 20.0%)\n x: array([1, 1, 0, 1, 1])\n y: array([1, 1, 0, 1, 0])')



======================================================================

FAIL: sklearn.tests.test_common.test_sample_weight_consistency(array([  3.35668635,   2.73091015,  13.93340473, -16.09503755,  -2.30172005]), array([  3.35668635,   2.73091015,  13.93340473, -15.33777767,  -2.30172005]), 6, 'ExtraTreesRegressor prediction not equal')

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/home/travis/build/scikit-learn/scikit-learn/testvenv/local/lib/python2.7/site-packages/nose/case.py", line 197, in runTest

    self.test(*self.arg)

  File "/usr/lib/python2.7/dist-packages/numpy/testing/utils.py", line 800, in assert_array_almost_equal

    header=('Arrays are not almost equal to %d decimals' % decimal))

  File "/usr/lib/python2.7/dist-packages/numpy/testing/utils.py", line 636, in assert_array_compare

    raise AssertionError(msg)

AssertionError: 

Arrays are not almost equal to 6 decimals

ExtraTreesRegressor prediction not equal

(mismatch 20.0%)

 x: array([  3.35668635,   2.73091015,  13.93340473, -16.09503755,  -2.30172005])

 y: array([  3.35668635,   2.73091015,  13.93340473, -15.33777767,  -2.30172005])

>>  raise AssertionError('\nArrays are not almost equal to 6 decimals\nExtraTreesRegressor prediction not equal\n(mismatch 20.0%)\n x: array([  3.35668635,   2.73091015,  13.93340473, -16.09503755,  -2.30172005])\n y: array([  3.35668635,   2.73091015,  13.93340473, -15.33777767,  -2.30172005])')

different errors on different machines I guess.
Also somehow we are causing a problem in test_get_params_invariance

ainafp · 2015-10-22T12:11:46Z

Do I exclude them too?

test_get_params_invariance has a typo

giorgiop · 2015-10-22T13:01:39Z

sklearn/tests/test_common.py

Can you make this casting more explicit, maybe with astype(np.int) ?

giorgiop · 2015-10-22T13:06:30Z

Is there any way to cover the case of float (positive) weights?

eickenberg · 2015-10-22T13:10:56Z

I guess one could work with rationals and then use the common denominator.
But the int case is an easy correspondence point to sample count and should
already catch the biggest problems, right?

On Thu, Oct 22, 2015 at 3:07 PM, Giorgio Patrini notifications@github.com
wrote:

Is there any way to cover the case of float (positive) weights?

—
Reply to this email directly or view it on GitHub
#5515 (comment)
.

giorgiop · 2015-10-22T13:36:35Z

sklearn/tests/test_common.py

Do you think a test for aug would make sense here? Something like

assert_equal(X_aug_train.shape[0], np.sum(sample_weight[train]))

giorgiop · 2015-10-22T14:29:02Z

@eickenberg Right, but that would fall on an implementation with integer numbers again. I agree that the tests with integers will uncover the biggest issues. However, it's true that most if not all the algorithms can take positive real number as weights.

giorgiop · 2015-10-23T12:04:19Z

sklearn/tests/test_common.py

I was not suggesting to do that. I think int makes more sense in this case.
My argument is that your tests will not cover the situation in which learning algorithms are weighted with float sample weights, as there is not a corresponding interpretation of "weight = number of sample copy". I simply do not think there is a way to cover this situation though.

well, you could have a dataset with some duplicate samples and use sample-weight 0.5 on it and compare against the data without duplicates? Or use 1.5 and compare it with data that has triples?

ainafp · 2015-10-23T12:23:21Z

ok I didn't understand it then.

Would it make sense to multiply the weights by the number of decimals used and then do the same (augment data)? Is this what you mean? It doesn't change much though.

giorgiop · 2015-10-23T12:37:46Z

I think solving weighting with rescaling (the multiplication by the weights) is only appropriate with linear models.

ainafp · 2015-10-23T13:02:42Z

What do you suggest then?

giorgiop · 2015-10-23T13:05:44Z

Your idea is the way to go with this test, I would not do anything else :)

…use it fails

ainafp · 2015-10-26T17:06:13Z

@ogrisel do you know why travis doesn't pass with python 3 but it does with python 2?

giorgiop · 2015-11-12T09:58:58Z

We were discussing in #5526 (comment) the addition of another test for sample_weight relative to linear models. The test is supposed to work under the conditions:

linear models fitted by square loss
regularization coefficient is 0 (if possibile) or very close to 0
X input matrix must describe an overdetermined linear system, i.e. X.shape[0] > X.shape[1]

The code should look like this one, looping over the appropriate regressors and testing both dense and sparse inputs.

DiegoVergara · 2016-09-01T15:02:51Z

How does it work sample weight in MultinomialNB ?, Is there any documentation or equation?

Someone can help me please, I appreciate it!

amueller · 2016-10-13T18:09:40Z

I think the issue is the same I saw in #7618. There should be an AttributeError, not a ValueError. That's a bug in the SVC.

amueller · 2016-10-13T18:10:00Z

Feel free to fix this here or do a separate PR.

amueller · 2016-12-07T23:42:50Z

fixes #5367.

amueller · 2016-12-07T23:45:02Z

Sorry I'm a bit out of the loop with this one. What's the status? Can you rebase?

jnothman · 2018-07-18T06:07:03Z

Hmm... I'd forgotten this was here. Should this be closed given #11558, @sergulaydore? I've not yet checked if it's completely redundant.

jnothman · 2018-07-18T06:07:09Z

Hmm... I'd forgotten this was here. Should this be closed given #11558, @sergulaydore? I've not yet checked if it's completely redundant.

sergulaydore · 2018-07-18T08:59:27Z

I think this is more relevant to #11598 which is not reviewed yet and also #9926.

eickenberg and others added 4 commits October 19, 2015 17:23

WIP adding common test for sample weights

23109f0

WIP testing structure in place, many tests failing. Proceeding to che…

7a0b752

…ck whether test should apply to all of the estimators

Added random_state to the estimator, and another test for weights 0 a…

5c882fa

…nd 1

Some prints removed

10da049

eickenberg mentioned this pull request Oct 21, 2015

[WIP] Common test for sample weight #5461

Closed

eickenberg reviewed Oct 21, 2015
View reviewed changes

ainafp added 2 commits October 21, 2015 19:03

Removed try: and added list of excluded estimators. Added parameters …

d789424

…for SGD estimators to change number of iterations or precision.

Exclusion list completed

0b2921a

ainafp added 2 commits October 22, 2015 14:12

fixed typo in test_get_params_invariance

f802d84

Added ExtraTreesRegressor and AdaBoostClassifier

ddc89ce

eickenberg mentioned this pull request Oct 22, 2015

[MRG + 1] Improve tests for sample_weight in LinearRegression and Ridge #5526

Merged

giorgiop reviewed Oct 22, 2015
View reviewed changes

sklearn/tests/test_common.py Outdated

Copy link
Copy Markdown

Contributor

giorgiop Oct 22, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this casting more explicit, maybe with astype(np.int) ?

Casting more explicit

984516e

giorgiop reviewed Oct 22, 2015
View reviewed changes

ainafp added 2 commits October 22, 2015 17:24

Added test for aug

e4e0848

Changed weights_samples to float

020b0e4

ainafp changed the title ~~Sample weight consistency~~ [MRG] Sample weight consistency Oct 22, 2015

ainafp changed the title ~~[MRG] Sample weight consistency~~ [WIP] Sample weight consistency Oct 22, 2015

Fix typo in test_get_params_invariance

da2a736

giorgiop reviewed Oct 23, 2015
View reviewed changes

Changed n_iter_ for iter in check_transformer_n_iter

c3ca2f0

ainafp added 4 commits October 23, 2015 17:14

Commented check_transformer_n_iter in test_get_params_invariance beca…

92a81e5

…use it fails

undo n_iter_ to n_iter

eedfada

Added case in which estimator has dual_coef_

ac281b5

Removed case in which estimator has dual_coef_

2f1ea6b

MechCoder mentioned this pull request Feb 8, 2016

Fix for passing pandas series in sample_weights in RidgeCV leading to an error(#5606) #6307

Closed

amueller added the Waiting for Reviewer label Oct 13, 2016

amueller added the Bug label Dec 7, 2016

amueller added this to the 0.19 milestone Dec 7, 2016

amueller mentioned this pull request Dec 7, 2016

[MRG + 1] raise AttributeError in SVC.coef_ for proper duck-typing #8009

Merged

amueller modified the milestone: 0.19 Jun 12, 2017

qinhanmin2014 mentioned this pull request Oct 17, 2017

Ensure that the shape of sample_weight is checked in all the functions #9926

Closed

rth self-requested a review February 5, 2020 21:19

rth mentioned this pull request Feb 21, 2020

Common check for sample weight invariance with removed samples #16507

Merged

rth closed this in #16507 May 10, 2020

cmarmo mentioned this pull request Dec 9, 2021

Test sample_weight in common tests #5367

Closed

Uh oh!

Conversation

ainafp commented Oct 21, 2015

Uh oh!

eickenberg Oct 21, 2015

Choose a reason for hiding this comment

Uh oh!

MechCoder Oct 21, 2015

Choose a reason for hiding this comment

Uh oh!

amueller commented Oct 21, 2015

Uh oh!

ainafp commented Oct 22, 2015

Uh oh!

eickenberg commented Oct 22, 2015

Uh oh!

ainafp commented Oct 22, 2015

Uh oh!

giorgiop Oct 22, 2015

Choose a reason for hiding this comment

Uh oh!

giorgiop commented Oct 22, 2015

Uh oh!

eickenberg commented Oct 22, 2015

Uh oh!

giorgiop Oct 22, 2015

Choose a reason for hiding this comment

Uh oh!

giorgiop commented Oct 22, 2015

Uh oh!

giorgiop Oct 23, 2015

Choose a reason for hiding this comment

Uh oh!

amueller Oct 13, 2016

Choose a reason for hiding this comment

Uh oh!

ainafp commented Oct 23, 2015

Uh oh!

giorgiop commented Oct 23, 2015

Uh oh!

ainafp commented Oct 23, 2015

Uh oh!

giorgiop commented Oct 23, 2015

Uh oh!

ainafp commented Oct 26, 2015

Uh oh!

giorgiop commented Nov 12, 2015

Uh oh!

DiegoVergara commented Sep 1, 2016

Uh oh!

amueller commented Oct 13, 2016

Uh oh!

amueller commented Oct 13, 2016

Uh oh!

amueller commented Dec 7, 2016

Uh oh!

amueller commented Dec 7, 2016

Uh oh!

jnothman commented Jul 18, 2018

Uh oh!

jnothman commented Jul 18, 2018

Uh oh!

sergulaydore commented Jul 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants