coverage increase

Author: janvanrijn

openml/flows/sklearn_converter.py

@@ -541,31 +541,32 @@ def model_single_core(model):
     Returns True if the parameter settings of model are chosen s.t. the model
      will run on a single core (in that case, openml-python can measure runtimes)
     '''
-    def check(param_dict):
+    def check(param_dict, disallow_parameter=False):
         for param, value in param_dict.items():
             # n_jobs is scikitlearn parameter for paralizing jobs
-            if 'n_jobs' in param.split('__')[-1]:
+            if param.split('__')[-1] == 'n_jobs':
                 # 0 = illegal value (?), 1 = use one core,  n = use n cores
                 # -1 = use all available cores -> this makes it hard to
                 # measure runtime in a fair way
-                if value != 1:
+                if value != 1 or disallow_parameter:
                     return False
         return True
 
     if not (isinstance(model, sklearn.base.BaseEstimator) or
             isinstance(model, sklearn.model_selection._search.BaseSearchCV)):
         raise ValueError('model should be BaseEstimator or BaseSearchCV')
 
-    # check the parameters for n_jobs
-    if check(model.get_params()) == False:
-        return False
-
     # check if the njobs is not in the optimization trace
     # this would be error by the user, so we can throw it as a courtesy
     if isinstance(model, sklearn.model_selection._search.BaseSearchCV):
-        if check(model.get_params()) == False:
+        if not check( model.param_grid, True):
             raise PyOpenMLError('openml-python should not be used to '
                                 'optimize the n_jobs parameter.')
+
+    # check the parameters for n_jobs
+    if check(model.get_params(), False) == False:
+        return False
+
     return True
 
 def _deserialize_cross_validator(value, **kwargs):

tests/test_flows/test_sklearn.py

@@ -558,16 +558,30 @@ def test_illegal_parameter_names_featureunion(self):
         self.assertRaises(ValueError, sklearn.pipeline.FeatureUnion, transformer_list=transformer_list)
 
     def test_paralizable_check(self):
-        models = [
+        singlecore_bagging = sklearn.ensemble.BaggingClassifier()
+        multicore_bagging = sklearn.ensemble.BaggingClassifier(n_jobs=5)
+        illegal_param_dist = {"base__n_jobs": [-1, 0, 1] }
+        legal_param_dist = {"base__max_depth": [2, 3, 4]}
+
+        legal_models = [
             sklearn.ensemble.RandomForestClassifier(),
             sklearn.ensemble.RandomForestClassifier(n_jobs=5),
             sklearn.ensemble.RandomForestClassifier(n_jobs=-1),
             sklearn.pipeline.Pipeline(steps=[('bag', sklearn.ensemble.BaggingClassifier(n_jobs=1))]),
             sklearn.pipeline.Pipeline(steps=[('bag', sklearn.ensemble.BaggingClassifier(n_jobs=5))]),
-            sklearn.pipeline.Pipeline(steps=[('bag', sklearn.ensemble.BaggingClassifier(n_jobs=-1))])
+            sklearn.pipeline.Pipeline(steps=[('bag', sklearn.ensemble.BaggingClassifier(n_jobs=-1))]),
+            sklearn.model_selection.GridSearchCV(singlecore_bagging, legal_param_dist),
+            sklearn.model_selection.GridSearchCV(multicore_bagging, legal_param_dist)
+        ]
+        illegal_models = [
+            sklearn.model_selection.GridSearchCV(singlecore_bagging, illegal_param_dist),
+            sklearn.model_selection.GridSearchCV(multicore_bagging, illegal_param_dist)
         ]
 
-        answers = [True, False, False, True, False, False]
+        answers = [True, False, False, True, False, False, True, False]
+
+        for i in range(len(legal_models)):
+            self.assertTrue(model_single_core(legal_models[i]) == answers[i])
 
-        for i in range(len(models)):
-            assert(model_single_core(models[i]) == answers[i])
+        for i in range(len(illegal_models)):
+            self.assertRaises(PyOpenMLError, model_single_core, illegal_models[i])