Make include_cls_metadata default to False for everything except Frameworks (#573)

When a HyperparameterTuner is created using attach(), the tuner needs to
pick an Estimator class to use. It looks for the following things (in this order):

1. if the hyperparameters have class metadata (e.g. a path like sagemaker.tensorflow.estimator.TensorFlow)
2. if the image being used corresponds to one of our 1P estimators
3. if nothing is present, in which case it just uses the generic Estimator class

This change helps the situation where people are using the
generic Estimator class with hyperparameter tuning.

Author: laurenyu

CHANGELOG.rst

@@ -15,6 +15,7 @@ CHANGELOG
 * doc-fix: Add information on necessary AWS permissions
 * bug-fix: Remove PyYAML to let docker-compose install the right version
 * enhancement: Add Model.transformer()
+* bug-fix: HyperparameterTuner: make ``include_cls_metadata`` default to ``False`` for everything except Frameworks
 
 1.16.3
 ======

README.rst

@@ -654,15 +654,9 @@ In addition, the ``fit()`` call uses a list of ``RecordSet`` objects instead of
     my_tuner.fit([train_records, test_records])
 
 To help attach a previously-started hyperparameter tuning job to a ``HyperparameterTuner`` instance,
-``fit()`` adds the module path of the class used to create the tuner to the list of static hyperparameters by default.
-If the algorithm you are using cannot handle unknown hyperparameters
-(for example, an Amazon SageMaker built-in algorithm that does not have a custom estimator in the Python SDK),
-set ``include_cls_metadata`` to ``False`` when you call ``fit``, so that it does not add the module path as a static hyperparameter:
-
-.. code:: python
-
-    my_tuner.fit({'train': 's3://my_bucket/my_training_data', 'test': 's3://my_bucket_my_testing_data'},
-                 include_cls_metadata=False)
+``fit()`` adds the module path of the class used to create the hyperparameter tuner to the list of static hyperparameters by default.
+If you are using your own custom estimator class (i.e. not one provided in this SDK) and want that class to be used when attaching a hyperparamter tuning job,
+set ``include_cls_metadata`` to ``True`` when you call ``fit`` to add the module path as static hyperparameters.
 
 There is also an analytics object associated with each ``HyperparameterTuner`` instance that contains useful information about the hyperparameter tuning job.
 For example, the ``dataframe`` method gets a pandas dataframe summarizing the associated training jobs:

src/sagemaker/tuner.py

@@ -18,7 +18,7 @@
 from enum import Enum
 
 import sagemaker
-from sagemaker.amazon.amazon_estimator import AmazonAlgorithmEstimatorBase, RecordSet
+from sagemaker.amazon.amazon_estimator import RecordSet
 from sagemaker.amazon.hyperparameter import Hyperparameter as hp  # noqa
 from sagemaker.analytics import HyperparameterTuningJobAnalytics
 from sagemaker.estimator import Framework
@@ -219,7 +219,7 @@ def __init__(self, estimator, objective_metric_name, hyperparameter_ranges, metr
         self.warm_start_config = warm_start_config
         self.early_stopping_type = early_stopping_type
 
-    def _prepare_for_training(self, job_name=None, include_cls_metadata=True):
+    def _prepare_for_training(self, job_name=None, include_cls_metadata=False):
         if job_name is not None:
             self._current_job_name = job_name
         else:
@@ -230,14 +230,14 @@ def _prepare_for_training(self, job_name=None, include_cls_metadata=True):
         for hyperparameter_name in self._hyperparameter_ranges.keys():
             self.static_hyperparameters.pop(hyperparameter_name, None)
 
-        # For attach() to know what estimator to use for non-1P algorithms
-        # (1P algorithms don't accept extra hyperparameters)
-        if include_cls_metadata and not isinstance(self.estimator, AmazonAlgorithmEstimatorBase):
+        # For attach() to know what estimator to use for frameworks
+        # (other algorithms may not accept extra hyperparameters)
+        if include_cls_metadata or isinstance(self.estimator, Framework):
             self.static_hyperparameters[self.SAGEMAKER_ESTIMATOR_CLASS_NAME] = json.dumps(
                 self.estimator.__class__.__name__)
             self.static_hyperparameters[self.SAGEMAKER_ESTIMATOR_MODULE] = json.dumps(self.estimator.__module__)
 
-    def fit(self, inputs=None, job_name=None, include_cls_metadata=True, **kwargs):
+    def fit(self, inputs=None, job_name=None, include_cls_metadata=False, **kwargs):
         """Start a hyperparameter tuning job.
 
         Args:
@@ -260,10 +260,12 @@ def fit(self, inputs=None, job_name=None, include_cls_metadata=True, **kwargs):
 
             job_name (str): Tuning job name. If not specified, the tuner generates a default job name,
                 based on the training image name and current timestamp.
-            include_cls_metadata (bool): Whether or not the hyperparameter tuning job should include information about
-                the estimator class (default: True). This information is passed as a hyperparameter, so if
-                the algorithm you are using cannot handle unknown hyperparameters (e.g. an Amazon ML algorithm that
-                does not have a custom estimator in the Python SDK), then set ``include_cls_metadata`` to ``False``.
+            include_cls_metadata (bool): Whether or not the hyperparameter tuning job should include
+                information about the estimator class (default: False). This information is passed
+                as a hyperparameter, so if the algorithm you are using cannot handle
+                unknown hyperparameters (e.g. an Amazon SageMaker built-in algorithm that
+                does not have a custom estimator in the Python SDK), then set
+                ``include_cls_metadata`` to ``False``.
             **kwargs: Other arguments needed for training. Please refer to the ``fit()`` method of the associated
                 estimator to see what other arguments are needed.
         """

tests/unit/test_tuner.py

@@ -13,22 +13,21 @@
 from __future__ import absolute_import
 
 import copy
-import json
-
 import os
+
 import pytest
 from mock import Mock
 
 from sagemaker import RealTimePredictor
-from sagemaker.amazon.pca import PCA
 from sagemaker.amazon.amazon_estimator import RecordSet
+from sagemaker.amazon.pca import PCA
 from sagemaker.estimator import Estimator
-from sagemaker.parameter import (ParameterRange, ContinuousParameter,
-                                 IntegerParameter, CategoricalParameter)
-from sagemaker.tuner import (HyperparameterTuner, _TuningJob, WarmStartConfig, WarmStartTypes,
-                             create_identical_dataset_and_algorithm_tuner,
-                             create_transfer_learning_tuner)
 from sagemaker.mxnet import MXNet
+from sagemaker.parameter import (CategoricalParameter, ContinuousParameter,
+                                 IntegerParameter, ParameterRange)
+from sagemaker.tuner import (_TuningJob, create_identical_dataset_and_algorithm_tuner,
+                             create_transfer_learning_tuner, HyperparameterTuner, WarmStartConfig,
+                             WarmStartTypes)
 
 DATA_DIR = os.path.join(os.path.dirname(__file__), '..', 'data')
 MODEL_DATA = "s3://bucket/model.tar.gz"
@@ -157,14 +156,9 @@ def test_prepare_for_training(tuner):
 
     assert tuner._current_job_name.startswith(IMAGE_NAME)
 
-    assert len(tuner.static_hyperparameters) == 3
+    assert len(tuner.static_hyperparameters) == 1
     assert tuner.static_hyperparameters['another_one'] == '0'
 
-    class_name = json.dumps(tuner.estimator.__class__.__name__)
-    assert tuner.static_hyperparameters['sagemaker_estimator_class_name'] == class_name
-    module = json.dumps(tuner.estimator.__module__)
-    assert tuner.static_hyperparameters['sagemaker_estimator_module'] == module
-
 
 def test_prepare_for_training_with_amazon_estimator(tuner, sagemaker_session):
     tuner.estimator = PCA(ROLE, TRAIN_INSTANCE_COUNT, TRAIN_INSTANCE_TYPE, NUM_COMPONENTS,
@@ -175,10 +169,10 @@ def test_prepare_for_training_with_amazon_estimator(tuner, sagemaker_session):
     assert 'sagemaker_estimator_module' not in tuner.static_hyperparameters
 
 
-def test_prepare_for_training_dont_include_estimator_cls(tuner):
-    tuner._prepare_for_training(include_cls_metadata=False)
-    assert 'sagemaker_estimator_class_name' not in tuner.static_hyperparameters
-    assert 'sagemaker_estimator_module' not in tuner.static_hyperparameters
+def test_prepare_for_training_include_estimator_cls(tuner):
+    tuner._prepare_for_training(include_cls_metadata=True)
+    assert 'sagemaker_estimator_class_name' in tuner.static_hyperparameters
+    assert 'sagemaker_estimator_module' in tuner.static_hyperparameters
 
 
 def test_prepare_for_training_with_job_name(tuner):