From e689caafb9d82d9b0560401ed39973922212aa0c Mon Sep 17 00:00:00 2001 From: Nitin Madnani Date: Wed, 13 Feb 2019 17:38:27 -0500 Subject: [PATCH 1/8] Add test for rescaling not applying to classifiers. - Flake8 fixes. --- tests/other/custom_learner.txt | 13 +++++++++++++ tests/other/test_load_saved_model.model | Bin 0 -> 1917 bytes tests/test_regression.py | 24 +++++++++++++++++------- 3 files changed, 30 insertions(+), 7 deletions(-) create mode 100644 tests/other/custom_learner.txt create mode 100644 tests/other/test_load_saved_model.model diff --git a/tests/other/custom_learner.txt b/tests/other/custom_learner.txt new file mode 100644 index 00000000..00ad0d87 --- /dev/null +++ b/tests/other/custom_learner.txt @@ -0,0 +1,13 @@ +# License: BSD 3 clause +""" +A simple wrapper around the existing LogisticRegression class, for testing +custom learners functionality. + +:author: Michael Heilman (mheilman@ets.org) +""" + +from sklearn.linear_model import LogisticRegression + + +class CustomLogisticRegressionWrapper(LogisticRegression): + pass diff --git a/tests/other/test_load_saved_model.model b/tests/other/test_load_saved_model.model new file mode 100644 index 0000000000000000000000000000000000000000..578faefc39bc1d2c4d9be5d56bde6b3e613fc9f7 GIT binary patch literal 1917 zcmah~&2tn*6yHq_U|4t?&}1PAxSuPoy9{;|a92Zab!&2)TbDy_pWN>D%VBp-p8F-FfGZ^8 zZcBvateI+&>Ld`xgza8Lc2@)w-3~nH z^!Q(Sfme1m4 zu%}G`7r17yw|$Eb22ZrTt!Tq4R!}qAgh>qh=+bC&Y_xd*_9KT5-csBbidp?Yvw$ZX zhz-%X_58L|6`Bl2FTnwOS{L3Xv`Z7iN?Vg@7%Ks=DCr0V2UiQC!WauvZZ}mjkB-8? znXVQyVqV)?C9wnJSeHWuOfzDC ziz1wt89dvjJ&p27B4#q4322?&@y*3`CD(~UUg0>&NQHixDV!h`Mf@rYF)W;PJ0xpT z2P6M5NvZHWOW+hGi+=nBTkAQuMx-xtO#!jPPWLES%q}OsfvGlmCpWg!&IGuPv76M$ zEDZzf1E+1f`h->D1xfvMAyyytJJA$Q?bET5Dt#Bw~)ijI_aL3wXI9r{y`>wnJ|T zUXi7{vMJ^c!}+o)F3f<3O(Fl?4zK#shWh!3-MueQf8TPy3O<>b>a~9SW%&KP@4MC) zd-q@X`nPXeZ-4aHpEsvIZ4t~{rH0oC$Hf9(U&V19E>$Qza{YVs$H!-uKKq=C5>o*g zyg_!XfH!Zm<9DDB$H6br2Q$$5Z}g%>Z%3!xCvEdli>{2$6(!D*HsT}}FkdRz4t*mO z&2o@;ekf*X|77=E7*8g7oH-C8*Ov>pQckxhj9Fk0K%v5IUl}{AP&pB06&H}KWHoRd z(kiPFVq<0W{NF#OGK8L)4Yra_85A(69TjE!vn#0Uc>}rt(~!sI2{|cG$+Dwzny=M( z)hjo%0&?n}$^m_~7UVefCBs4Qb3arej)}#dx7`NHjbI%S<8Ot;z|~%vI8AO1-FEIT H!L{Z;sastz literal 0 HcmV?d00001 diff --git a/tests/test_regression.py b/tests/test_regression.py index c090082c..c1ea1066 100644 --- a/tests/test_regression.py +++ b/tests/test_regression.py @@ -21,18 +21,19 @@ from itertools import product from os.path import abspath, dirname, join, exists -from nose.tools import eq_, assert_almost_equal +from nose.tools import eq_, assert_almost_equal, raises import numpy as np from numpy.testing import assert_allclose from scipy.stats import pearsonr from sklearn.exceptions import ConvergenceWarning +from sklearn.linear_model import LogisticRegression from sklearn.utils.testing import assert_greater, assert_less from skll.data import FeatureSet, NDJWriter from skll.config import _setup_config_parser from skll.experiments import run_configuration -from skll.learner import Learner +from skll.learner import Learner, rescaled from skll.learner import _DEFAULT_PARAM_GRIDS from utils import make_regression_data, fill_in_config_paths_for_fancy_output @@ -676,10 +677,19 @@ def test_dummy_regressor_predict(): {"strategy": "quantile", "quantile": 0.0}, {"strategy": "quantile", "quantile": 1.0}, {"strategy": "constant", "constant": 1}], - [np.ones(10)*np.mean(train_labels), - np.ones(10)*np.median(train_labels), - np.ones(10)*np.median(train_labels), - np.ones(10)*np.min(train_labels), - np.ones(10)*np.max(train_labels), + [np.ones(10) * np.mean(train_labels), + np.ones(10) * np.median(train_labels), + np.ones(10) * np.median(train_labels), + np.ones(10) * np.min(train_labels), + np.ones(10) * np.max(train_labels), np.ones(10)]): yield check_dummy_regressor_predict, model_args, train_labels, expected_output + + +@raises(ValueError) +def test_learner_api_rescaling_classifier(): + """ + Check that rescaling fails for classifiers + """ + + _ = rescaled(LogisticRegression) From 63b71b45768947db8c143290dd750990a3fcf90b Mon Sep 17 00:00:00 2001 From: Nitin Madnani Date: Wed, 13 Feb 2019 17:39:26 -0500 Subject: [PATCH 2/8] Add new tests - Test for binary classification with with a correlation metric. - Test for the `_train_and_score()` global function. - Test for the `Learner.load()` method. - Remove unnecessary import --- tests/test_classification.py | 72 +++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/tests/test_classification.py b/tests/test_classification.py index 56ffc610..89ac4a71 100644 --- a/tests/test_classification.py +++ b/tests/test_classification.py @@ -26,7 +26,6 @@ from nose.tools import eq_, assert_almost_equal, raises from sklearn.exceptions import ConvergenceWarning -from sklearn.feature_extraction import FeatureHasher from sklearn.metrics import accuracy_score from skll.data import FeatureSet @@ -34,7 +33,7 @@ from skll.data.writers import NDJWriter from skll.config import _parse_config_file from skll.experiments import run_configuration -from skll.learner import Learner +from skll.learner import Learner, _train_and_score from skll.learner import _DEFAULT_PARAM_GRIDS from utils import (make_classification_data, make_regression_data, @@ -288,6 +287,17 @@ def test_mlp_classification(): assert_almost_equal(accuracy, 0.858, places=3) +def test_binary_classification_with_correlation_metric(): + train_fs, test_fs = make_classification_data(num_examples=600, + train_test_ratio=0.7, + num_features=5) + learner = Learner('LogisticRegression') + train_score = learner.train(train_fs, + grid_search=True, + grid_objective='pearson') + assert_almost_equal(train_score, 0.5265, places=4) + + def check_sparse_predict_sampler(use_feature_hashing=False): train_fs, test_fs = make_sparse_data( use_feature_hashing=use_feature_hashing) @@ -671,3 +681,61 @@ def test_bad_xval_float_classes(): yield check_bad_xval_float_classes, True yield check_bad_xval_float_classes, False + + +def test_train_and_score_function(): + """ + Check that the _train_and_score() function works as expected + """ + + # create train and test data + (train_fs, + test_fs) = make_classification_data(num_examples=500, + train_test_ratio=0.7, + num_features=5, + use_feature_hashing=False, + non_negative=True) + + # call _train_and_score() on this data + learner1 = Learner('LogisticRegression') + train_score1, test_score1 = _train_and_score(learner1, train_fs, test_fs, 'accuracy') + + # this should yield identical results when training another instance + # of the same learner without grid search and shuffling and evaluating + # that instance on the train and the test set + learner2 = Learner("LogisticRegression") + learner2.train(train_fs, grid_search=False, shuffle=False) + train_score2 = learner2.evaluate(train_fs, output_metrics=['accuracy'])[-1]['accuracy'] + test_score2 = learner2.evaluate(test_fs, output_metrics=['accuracy'])[-1]['accuracy'] + + eq_(train_score1, train_score2) + eq_(test_score1, test_score2) + + +def test_learner_api_load_into_existing_instance(): + """ + Check that `Learner.load()` works as expected + """ + + # create a LinearSVC instance and train it on some data + learner1 = Learner('LinearSVC') + (train_fs, + test_fs) = make_classification_data(num_examples=200, + num_features=5, + use_feature_hashing=False, + non_negative=True) + learner1.train(train_fs, grid_search=False) + + # now use `load()` to replace the existing instance with a + # different saved learner + other_model_file = join(_my_dir, 'other', 'test_load_saved_model.model') + learner1.load(other_model_file) + + # now load the saved model into another instance using the class method + # `from_file()` + learner2 = Learner.from_file(other_model_file) + + # check that the two instances are now basically the same + eq_(learner1.model_type, learner2.model_type) + eq_(learner1.model_params, learner2.model_params) + eq_(learner1.model_kwargs, learner2.model_kwargs) From 43ad0cfd3296f58a115f33600616a29332fe3c7e Mon Sep 17 00:00:00 2001 From: Nitin Madnani Date: Wed, 13 Feb 2019 17:39:40 -0500 Subject: [PATCH 3/8] Add custom learner API tests --- tests/test_custom_learner.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tests/test_custom_learner.py b/tests/test_custom_learner.py index ada36c5a..0c4c732e 100644 --- a/tests/test_custom_learner.py +++ b/tests/test_custom_learner.py @@ -19,10 +19,11 @@ from os.path import abspath, dirname, exists, join import numpy as np +from nose.tools import raises from numpy.testing import assert_array_equal from skll.data import NDJWriter from skll.experiments import run_configuration -from skll.learner import _DEFAULT_PARAM_GRIDS +from skll.learner import _DEFAULT_PARAM_GRIDS, Learner from utils import fill_in_config_paths, make_classification_data @@ -204,8 +205,7 @@ def test_custom_learner_model_loading(): outprefix = 'test_model_custom_learner' pred_file = join(_my_dir, 'output', '{}_{}_CustomLogisticRegressionWrapper' - '_predictions.tsv'.format(outprefix, - outprefix)) + '_predictions.tsv'.format(outprefix, outprefix)) preds1 = read_predictions(pred_file) os.unlink(pred_file) @@ -222,3 +222,14 @@ def test_custom_learner_model_loading(): # make sure that they are the same as before assert_array_equal(preds1, preds2) + + +@raises(ValueError) +def test_custom_learner_api_missing_file(): + _ = Learner('CustomType') + + +@raises(ValueError) +def test_custom_learner_api_bad_extension(): + other_dir = join(_my_dir, 'other') + _ = Learner('_CustomLogisticRegressionWrapper', custom_learner_path=join(other_dir, 'custom_learner.txt')) From 407a0722ccebecc3797e4bcf3827ca69206d8d0c Mon Sep 17 00:00:00 2001 From: Nitin Madnani Date: Wed, 13 Feb 2019 17:39:55 -0500 Subject: [PATCH 4/8] Make error method more consistent with default values. --- skll/learner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skll/learner.py b/skll/learner.py index 51756272..874dfcdd 100644 --- a/skll/learner.py +++ b/skll/learner.py @@ -1386,7 +1386,7 @@ def train(self, examples, param_grid=None, grid_search_folds=3, if grid_search: if not grid_objective: raise ValueError("You must specify a grid objective " - "if doing grid search.") + "or turn off grid search.") if self.model_type._estimator_type == 'regressor': # types 2-4 are valid for all regression models if grid_objective in _CLASSIFICATION_ONLY_OBJ_FUNCS: From 5dbbd6947e29f1784e6d5402245bb714703ca448 Mon Sep 17 00:00:00 2001 From: Nitin Madnani Date: Wed, 13 Feb 2019 18:27:05 -0500 Subject: [PATCH 5/8] Accommodate python 2.7 for Learner.load() test --- tests/other/test_load_saved_model.2.model | Bin 0 -> 1787 bytes ...el.model => test_load_saved_model.3.model} | Bin 1917 -> 1917 bytes tests/test_classification.py | 3 ++- 3 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 tests/other/test_load_saved_model.2.model rename tests/other/{test_load_saved_model.model => test_load_saved_model.3.model} (81%) diff --git a/tests/other/test_load_saved_model.2.model b/tests/other/test_load_saved_model.2.model new file mode 100644 index 0000000000000000000000000000000000000000..b833aa2d803e396f029d4a9613b51cc1355a9790 GIT binary patch literal 1787 zcmah}&2tn*6yHs<`IvkH0*ZY3$wp-fAwX1cg`j9AteV)cirDtd^llHCuQ%P3u;_>$ z0!fuO<>j?2pnr{&XJFuL{qItCk=}q_4lM~rgY+%&*a{_w zL9;}LTdtyRpm=QaNWsu5>6M{}Ct$cafS-DMAYJY>Y&T10`3CHmhn-ck zo0Lz$gN;UG_AXx8%`7Q+$RZUf7^c`|cCEm}q!Pvs!rUzeNWV<13>)kz(gzOHI1JcJ z4CZ;^GWC5H!M@eR$_Xiz%Ef~81=Pj*DeM;-?jtEYS{2)bhBgI{<UZ zfF*XBcCT^%bSF!$)KNj%B6V` z&ouS7EKwmGBttO^oG{Z>aj4C1|Ndeh8KZ?!C@Q6W0EbBLL~U}Sb{Hl|kCQMD4zH1J z6?!mvw!Z7wuYc~IIY=sY$XgVSWOYuZaI~niH4Vq|I*-r8bW!KC_tp7aR;O9H4$p6@ z^tan%`#%5dms|h*{?q;)KYmfaGyct;uWx>N>+bBxA1lYds~?#A^sB!v->wtm9vg5X z1938i7c_|JoWc60yjOns54#7@%h6h4Oe>##5n zZ;C?#EntUH2gn+7Gpy^!gdVq57{mF^(`X@!pYEJlIV&DxjnsKs}G2&iDsarWKTG^W8 z+6wzhv!&k|XK;W1-WJ+OkVwI;k{%i2ti8NSM%FzRFkdsYcnQ2}vm0@=CJuvNtR^8S zR!1N#R#OlatD^v%3`icam}#M6Qa%Y1&4K|O)@qduE`>I;5|q9}AxUAWD!}X!k=ZSL zvm`>ZO9W;)5$iLy<%LWgn=)V-X&$2JV2fF<=i=PCvPQPiL~`oz0Ph8S2_1A{VZxXe k0$feueH1@)W(*%R*1Jvf1ZfH%>Tbh+!(S)Xb+}gh7aU$>=l}o! literal 0 HcmV?d00001 diff --git a/tests/other/test_load_saved_model.model b/tests/other/test_load_saved_model.3.model similarity index 81% rename from tests/other/test_load_saved_model.model rename to tests/other/test_load_saved_model.3.model index 578faefc39bc1d2c4d9be5d56bde6b3e613fc9f7..7ec2a49752ef58c8398374675b0e8bdf3d624221 100644 GIT binary patch delta 125 zcmey%_m^*j3KLTb!)7%mLq_%w3}CQ1j`;?o`lE9K@^hy=+<*4zZ3VIG3+)-S_TJes zb>n^>Z@y=(=11%$eI{>vlXY%#EUO8}qKiHEvZo%h-#n9*jZrBDVhjUrdh$WGJpdp) BCxie1 delta 125 zcmey%_m^*j3KP>3#?5L>hK%ejRS(_QiBxZnW4^(t{`8)F=P|b%_TI;ncbmC)+CO+H zGP9KFn*AXy-S9JSF4@ Date: Wed, 13 Feb 2019 20:16:27 -0500 Subject: [PATCH 6/8] Remove unnecessary test. --- tests/test_classification.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tests/test_classification.py b/tests/test_classification.py index c557e918..bde537ce 100644 --- a/tests/test_classification.py +++ b/tests/test_classification.py @@ -288,17 +288,6 @@ def test_mlp_classification(): assert_almost_equal(accuracy, 0.858, places=3) -def test_binary_classification_with_correlation_metric(): - train_fs, test_fs = make_classification_data(num_examples=600, - train_test_ratio=0.7, - num_features=5) - learner = Learner('LogisticRegression') - train_score = learner.train(train_fs, - grid_search=True, - grid_objective='pearson') - assert_almost_equal(train_score, 0.5265, places=4) - - def check_sparse_predict_sampler(use_feature_hashing=False): train_fs, test_fs = make_sparse_data( use_feature_hashing=use_feature_hashing) From c23738bbec004fa80c6f76e3885446f5ed7d53be Mon Sep 17 00:00:00 2001 From: Nitin Madnani Date: Thu, 14 Feb 2019 08:46:34 -0500 Subject: [PATCH 7/8] Tweak and add tests - Run the _train_and_score() test for both classifiers and regressors. - Add a new test for calling learner.train() without specifying grid objective. --- tests/test_classification.py | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/tests/test_classification.py b/tests/test_classification.py index bde537ce..5aba3880 100644 --- a/tests/test_classification.py +++ b/tests/test_classification.py @@ -673,7 +673,7 @@ def test_bad_xval_float_classes(): yield check_bad_xval_float_classes, False -def test_train_and_score_function(): +def check_train_and_score_function(model_type): """ Check that the _train_and_score() function works as expected """ @@ -687,21 +687,41 @@ def test_train_and_score_function(): non_negative=True) # call _train_and_score() on this data - learner1 = Learner('LogisticRegression') - train_score1, test_score1 = _train_and_score(learner1, train_fs, test_fs, 'accuracy') + estimator_name = 'LogisticRegression' if model_type == 'classifier' else 'Ridge' + metric = 'accuracy' if model_type == 'classifier' else 'pearson' + learner1 = Learner(estimator_name) + train_score1, test_score1 = _train_and_score(learner1, train_fs, test_fs, metric) # this should yield identical results when training another instance # of the same learner without grid search and shuffling and evaluating # that instance on the train and the test set - learner2 = Learner("LogisticRegression") + learner2 = Learner(estimator_name) learner2.train(train_fs, grid_search=False, shuffle=False) - train_score2 = learner2.evaluate(train_fs, output_metrics=['accuracy'])[-1]['accuracy'] - test_score2 = learner2.evaluate(test_fs, output_metrics=['accuracy'])[-1]['accuracy'] + train_score2 = learner2.evaluate(train_fs, output_metrics=[metric])[-1][metric] + test_score2 = learner2.evaluate(test_fs, output_metrics=[metric])[-1][metric] eq_(train_score1, train_score2) eq_(test_score1, test_score2) +def test_train_and_score_function(): + yield check_train_and_score_function, 'classifier' + yield check_train_and_score_function, 'regressor' + + +@raises(ValueError) +def test_learner_api_grid_search_no_objective(): + + (train_fs, + test_fs) = make_classification_data(num_examples=500, + train_test_ratio=0.7, + num_features=5, + use_feature_hashing=False, + non_negative=True) + learner = Learner('LogisticRegression') + _ = learner.train(train_fs) + + def test_learner_api_load_into_existing_instance(): """ Check that `Learner.load()` works as expected From 021d250c18ae61738f2a72b8d65680c42cb03e45 Mon Sep 17 00:00:00 2001 From: Nitin Madnani Date: Thu, 14 Feb 2019 14:53:53 -0500 Subject: [PATCH 8/8] Make error message more verbose. --- skll/learner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/skll/learner.py b/skll/learner.py index 874dfcdd..c038cd0c 100644 --- a/skll/learner.py +++ b/skll/learner.py @@ -1385,8 +1385,8 @@ def train(self, examples, param_grid=None, grid_search_folds=3, # selected learner if grid_search: if not grid_objective: - raise ValueError("You must specify a grid objective " - "or turn off grid search.") + raise ValueError("Grid search is on by default. You must either " + "specify a grid objective or turn off grid search.") if self.model_type._estimator_type == 'regressor': # types 2-4 are valid for all regression models if grid_objective in _CLASSIFICATION_ONLY_OBJ_FUNCS: