fix datapreprocessing clonability (#925)

franchuterivera · web-flow · commit 07b8f2ef9919 · 2020-08-21T17:57:51.000+02:00
* fix datapreprocessing clonability

* Fix check is fitted

* Feedback from comments

* Enforce is fitted to every pipeline stage
diff --git a/autosklearn/automl.py b/autosklearn/automl.py
@@ -650,6 +650,11 @@ def predict(self, X, batch_size=None, n_jobs=1):
                     check_is_fitted(tmp_model.steps[-1][-1])
             models = self.models_
         except sklearn.exceptions.NotFittedError:
+            # When training a cross validation model, self.cv_models_
+            # will contain the Voting classifier/regressor product of cv
+            # self.models_ in the case of cv, contains unfitted models
+            # Raising above exception is a mechanism to detect which
+            # attribute contains the relevant models for prediction
             try:
                 check_is_fitted(list(self.cv_models_.values())[0])
                 models = self.cv_models_
diff --git a/autosklearn/pipeline/components/data_preprocessing/balancing/balancing.py b/autosklearn/pipeline/components/data_preprocessing/balancing/balancing.py
@@ -14,6 +14,7 @@ def __init__(self, strategy='none', random_state=None):
         self.random_state = random_state
 
     def fit(self, X, y=None):
+        self.fitted_ = True
         return self
 
     def transform(self, X):
diff --git a/autosklearn/pipeline/components/data_preprocessing/categorical_encoding/no_encoding.py b/autosklearn/pipeline/components/data_preprocessing/categorical_encoding/no_encoding.py
@@ -9,6 +9,7 @@ def __init__(self, random_state=None):
         pass
 
     def fit(self, X, y=None):
+        self.fitted_ = True
         return self
 
     def transform(self, X):
diff --git a/autosklearn/pipeline/components/data_preprocessing/data_preprocessing.py b/autosklearn/pipeline/components/data_preprocessing/data_preprocessing.py
@@ -22,7 +22,8 @@ class DataPreprocessor(AutoSklearnComponent):
 
     def __init__(self, config=None, pipeline=None, dataset_properties=None, include=None,
                  exclude=None, random_state=None, init_params=None,
-                 categorical_features=None, force_sparse_output=False):
+                 categorical_features=None, force_sparse_output=False,
+                 column_transformer=None):
 
         if pipeline is not None:
             raise ValueError("DataPreprocessor's argument 'pipeline' should be None")
@@ -70,6 +71,7 @@ def __init__(self, config=None, pipeline=None, dataset_properties=None, include=
         ]
         if self.config:
             self.set_hyperparameters(self.config, init_params=init_params)
+        self.column_transformer = column_transformer
 
     def fit(self, X, y=None):
 
@@ -105,7 +107,10 @@ def fit(self, X, y=None):
 
     def transform(self, X):
         if self.column_transformer is None:
-            raise NotImplementedError()
+            raise ValueError("Cannot call transform on a Datapreprocessor that has not"
+                             "yet been fit. Please check the log files for errors "
+                             "while trying to fit the model."
+                             )
         return self.column_transformer.transform(X)
 
     def fit_transform(self, X, y=None):
diff --git a/autosklearn/pipeline/components/data_preprocessing/data_preprocessing_categorical.py b/autosklearn/pipeline/components/data_preprocessing/data_preprocessing_categorical.py
@@ -83,7 +83,6 @@ def _get_hyperparameter_search_space(self, include=None, exclude=None,
             cs=cs, dataset_properties=dataset_properties,
             exclude=exclude, include=include, pipeline=self.steps)
 
-        self.configuration_space_ = cs
         return cs
 
     def _get_pipeline_steps(self, dataset_properties=None):
diff --git a/autosklearn/pipeline/components/data_preprocessing/data_preprocessing_numerical.py b/autosklearn/pipeline/components/data_preprocessing/data_preprocessing_numerical.py
@@ -85,7 +85,6 @@ def _get_hyperparameter_search_space(self, include=None, exclude=None,
             cs=cs, dataset_properties=dataset_properties,
             exclude=exclude, include=include, pipeline=self.steps)
 
-        self.configuration_space_ = cs
         return cs
 
     def _get_pipeline_steps(self, dataset_properties=None):
diff --git a/autosklearn/pipeline/components/feature_preprocessing/__init__.py b/autosklearn/pipeline/components/feature_preprocessing/__init__.py
@@ -121,8 +121,6 @@ def get_hyperparameter_search_space(self, dataset_properties=None,
             cs.add_configuration_space(name, preprocessor_configuration_space,
                                        parent_hyperparameter=parent_hyperparameter)
 
-        self.configuration_space_ = cs
-        self.dataset_properties_ = dataset_properties
         return cs
 
     def transform(self, X):
diff --git a/autosklearn/pipeline/components/feature_preprocessing/densifier.py b/autosklearn/pipeline/components/feature_preprocessing/densifier.py
@@ -9,6 +9,7 @@ def __init__(self, random_state=None):
         pass
 
     def fit(self, X, y=None):
+        self.fitted_ = True
         return self
 
     def transform(self, X):
diff --git a/autosklearn/pipeline/components/feature_preprocessing/no_preprocessing.py b/autosklearn/pipeline/components/feature_preprocessing/no_preprocessing.py
@@ -12,6 +12,7 @@ def __init__(self, random_state):
 
     def fit(self, X, Y=None):
         self.preprocessor = 0
+        self.fitted_ = True
         return self
 
     def transform(self, X):
diff --git a/autosklearn/pipeline/components/regression/__init__.py b/autosklearn/pipeline/components/regression/__init__.py
@@ -108,8 +108,6 @@ def get_hyperparameter_search_space(self, dataset_properties=None,
             cs.add_configuration_space(estimator_name, estimator_configuration_space,
                                        parent_hyperparameter=parent_hyperparameter)
 
-        self.configuration_space_ = cs
-        self.dataset_properties_ = dataset_properties
         return cs
 
     def estimator_supports_iterative_fit(self):
diff --git a/test/test_automl/test_estimators.py b/test/test_automl/test_estimators.py
@@ -711,6 +711,33 @@ def test_regression(self):
         # On average np.sqrt(30) away from the target -> ~5.5 on average
         self.assertGreaterEqual(score, -30)
 
+    def test_cv_regression(self):
+        """
+        Makes sure that when using a cv strategy, we are able to fit
+        a regressor
+        """
+        tmp = os.path.join(self.test_dir, '..', '.tmp_regression_fit')
+        output = os.path.join(self.test_dir, '..', '.out_regression_fit')
+        self._setUp(tmp)
+        self._setUp(output)
+
+        X_train, Y_train, X_test, Y_test = putil.get_dataset('boston')
+        automl = AutoSklearnRegressor(time_left_for_this_task=30,
+                                      per_run_time_limit=5,
+                                      resampling_strategy='cv',
+                                      tmp_folder=tmp,
+                                      output_folder=output)
+
+        automl.fit(X_train, Y_train)
+        predictions = automl.predict(X_test)
+        self.assertEqual(predictions.shape, (356,))
+        score = mean_squared_error(Y_test, predictions)
+        # On average np.sqrt(30) away from the target -> ~5.5 on average
+        self.assertGreaterEqual(score, -30)
+
+        self._tearDown(tmp)
+        self._tearDown(output)
+
     def test_regression_pandas_support(self):
         X, y = sklearn.datasets.fetch_openml(
             data_id=41514,  # diabetes
diff --git a/test/test_pipeline/test_classification.py b/test/test_pipeline/test_classification.py
@@ -16,6 +16,7 @@
 import sklearn.model_selection
 import sklearn.ensemble
 import sklearn.svm
+from sklearn.utils.validation import check_is_fitted
 
 from ConfigSpace.configuration_space import ConfigurationSpace
 from ConfigSpace.hyperparameters import CategoricalHyperparameter
@@ -334,8 +335,28 @@ def _test_configurations(self, configurations_space, make_sparse=False,
                                                dataset_properties=dataset_properties,
                                                init_params=init_params_,)
             cls.set_hyperparameters(config, init_params=init_params_)
+
+            # First make sure that for this configuration, setting the parameters
+            # does not mistakenly set the estimator as fitted
+            for name, step in cls.named_steps.items():
+                with self.assertRaisesRegex(sklearn.exceptions.NotFittedError,
+                                            "instance is not fitted yet"):
+                    check_is_fitted(step)
+
             try:
                 cls.fit(X_train, Y_train)
+
+                # After fit, all components should be tagged as fitted
+                # by sklearn. Check is fitted raises an exception if that
+                # is not the case
+                try:
+                    for name, step in cls.named_steps.items():
+                        check_is_fitted(step)
+                except sklearn.exceptions.NotFittedError:
+                    self.fail("config={} raised NotFittedError unexpectedly!".format(
+                        config
+                    ))
+
                 cls.predict(X_test.copy())
                 cls.predict_proba(X_test)
             except MemoryError:
@@ -720,7 +741,7 @@ def test_set_hyperparameters_honors_configuration(self):
         configuration from Config was checked
         """
 
-        all_combinations = list(itertools.combinations([True, False], r=4))
+        all_combinations = list(itertools.product([True, False], repeat=4))
         for sparse, multilabel, signed, multiclass, in all_combinations:
             dataset_properties = {
                 'sparse': sparse,
diff --git a/test/test_pipeline/test_regression.py b/test/test_pipeline/test_regression.py
@@ -14,6 +14,7 @@
 from sklearn.base import clone
 import sklearn.ensemble
 import sklearn.svm
+from sklearn.utils.validation import check_is_fitted
 
 from ConfigSpace.configuration_space import ConfigurationSpace
 from ConfigSpace.hyperparameters import CategoricalHyperparameter
@@ -168,8 +169,27 @@ def _test_configurations(self, configurations_space, make_sparse=False,
             cls = SimpleRegressionPipeline(random_state=1,
                                            dataset_properties=dataset_properties)
             cls.set_hyperparameters(config)
+
+            # First make sure that for this configuration, setting the parameters
+            # does not mistakenly set the estimator as fitted
+            for name, step in cls.named_steps.items():
+                with self.assertRaisesRegex(sklearn.exceptions.NotFittedError,
+                                            "instance is not fitted yet"):
+                    check_is_fitted(step)
+
             try:
                 cls.fit(X_train, Y_train)
+                # After fit, all components should be tagged as fitted
+                # by sklearn. Check is fitted raises an exception if that
+                # is not the case
+                try:
+                    for name, step in cls.named_steps.items():
+                        check_is_fitted(step)
+                except sklearn.exceptions.NotFittedError:
+                    self.fail("config={} raised NotFittedError unexpectedly!".format(
+                        config
+                    ))
+
                 cls.predict(X_test)
             except MemoryError:
                 continue
@@ -502,7 +522,7 @@ def test_set_hyperparameters_honors_configuration(self):
         configuration from Config was checked
         """
 
-        all_combinations = list(itertools.combinations([True, False], r=4))
+        all_combinations = list(itertools.product([True, False], repeat=4))
         for sparse, multilabel, signed, multiclass, in all_combinations:
             dataset_properties = {
                 'sparse': sparse,
