PR comments

mvsusp · mvsusp · commit 4e02b12d0edb · 2018-11-18T17:50:13.000-08:00
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -5,7 +5,7 @@ CHANGELOG
 1.15.1dev
 =========
 
-* feature: Estimators: lib_dirs attribute allows export of additional libraries into the container
+* feature: Estimators: dependencies attribute allows export of additional libraries into the container
 * feature: Add APIs to export Airflow transform and deploy config
 
 1.15.0
diff --git a/src/sagemaker/chainer/README.rst b/src/sagemaker/chainer/README.rst
@@ -149,14 +149,14 @@ The following are optional arguments. When you create a ``Chainer`` object, you
    other training source code dependencies including the entry point
    file. Structure within this directory will be preserved when training
    on SageMaker.
-- ``lib_dirs (list[str])`` A list of paths to directories (absolute or relative) with
+- ``dependencies (list[str])`` A list of paths to directories (absolute or relative) with
         any additional libraries that will be exported to the container (default: []).
         The library folders will be copied to SageMaker in the same folder where the entrypoint is copied.
         If the ```source_dir``` points to S3, code will be uploaded and the S3 location will be used
         instead. Example:
 
             The following call
-            >>> Estimator(entry_point='train.py', lib_dirs=['my/libs/common', 'virtual-env'])
+            >>> Chainer(entry_point='train.py', dependencies=['my/libs/common', 'virtual-env'])
             results in the following inside the container:
 
             >>> $ ls
diff --git a/src/sagemaker/chainer/estimator.py b/src/sagemaker/chainer/estimator.py
@@ -133,7 +133,7 @@ def create_model(self, model_server_workers=None, role=None, vpc_config_override
                             py_version=self.py_version, framework_version=self.framework_version,
                             model_server_workers=model_server_workers, image=self.image_name,
                             sagemaker_session=self.sagemaker_session,
-                            vpc_config=self.get_vpc_config(vpc_config_override), lib_dirs=self.lib_dirs)
+                            vpc_config=self.get_vpc_config(vpc_config_override), dependencies=self.dependencies)
 
     @classmethod
     def _prepare_init_params_from_job_description(cls, job_details, model_channel_name=None):
diff --git a/src/sagemaker/estimator.py b/src/sagemaker/estimator.py
@@ -637,7 +637,7 @@ class Framework(EstimatorBase):
     LAUNCH_PS_ENV_NAME = 'sagemaker_parameter_server_enabled'
 
     def __init__(self, entry_point, source_dir=None, hyperparameters=None, enable_cloudwatch_metrics=False,
-                 container_log_level=logging.INFO, code_location=None, image_name=None, lib_dirs=None, **kwargs):
+                 container_log_level=logging.INFO, code_location=None, image_name=None, dependencies=None, **kwargs):
         """Base class initializer. Subclasses which override ``__init__`` should invoke ``super()``
 
         Args:
@@ -646,13 +646,13 @@ def __init__(self, entry_point, source_dir=None, hyperparameters=None, enable_cl
             source_dir (str): Path (absolute or relative) to a directory with any other training
                 source code dependencies aside from tne entry point file (default: None). Structure within this
                 directory are preserved when training on Amazon SageMaker.
-            lib_dirs (list[str]): A list of paths to directories (absolute or relative) with
+            dependencies (list[str]): A list of paths to directories (absolute or relative) with
                 any additional libraries that will be exported to the container (default: []).
                 The library folders will be copied to SageMaker in the same folder where the entrypoint is copied.
                 Example:
 
                     The following call
-                    >>> Estimator(entry_point='train.py', lib_dirs=['my/libs/common', 'virtual-env'])
+                    >>> Estimator(entry_point='train.py', dependencies=['my/libs/common', 'virtual-env'])
                     results in the following inside the container:
 
                     >>> $ ls
@@ -679,7 +679,7 @@ def __init__(self, entry_point, source_dir=None, hyperparameters=None, enable_cl
         """
         super(Framework, self).__init__(**kwargs)
         self.source_dir = source_dir
-        self.lib_dirs = lib_dirs or []
+        self.dependencies = dependencies or []
         self.entry_point = entry_point
         if enable_cloudwatch_metrics:
             warnings.warn('enable_cloudwatch_metrics is now deprecated and will be removed in the future.',
@@ -747,7 +747,7 @@ def _stage_user_code_in_s3(self):
                                   s3_key_prefix=code_s3_prefix,
                                   script=self.entry_point,
                                   directory=self.source_dir,
-                                  lib_dirs=self.lib_dirs)
+                                  dependencies=self.dependencies)
 
     def _model_source_dir(self):
         """Get the appropriate value to pass as source_dir to model constructor on deploying
diff --git a/src/sagemaker/fw_utils.py b/src/sagemaker/fw_utils.py
@@ -111,7 +111,7 @@ def validate_source_dir(script, directory):
     return True
 
 
-def tar_and_upload_dir(session, bucket, s3_key_prefix, script, directory, lib_dirs=None):
+def tar_and_upload_dir(session, bucket, s3_key_prefix, script, directory, dependencies=None):
     """Pack and upload source files to S3 only if directory is empty or local.
 
     Note:
@@ -124,14 +124,14 @@ def tar_and_upload_dir(session, bucket, s3_key_prefix, script, directory, lib_di
         script (str): Script filename.
         directory (str or None): Directory containing the source file. If it starts with
                                     "s3://", no action is taken.
-        lib_dirs (List[str]): A list of paths to directories (absolute or relative)
+        dependencies (List[str]): A list of paths to directories (absolute or relative)
                                 containing additional libraries that will be copied into
                                 /opt/ml/lib
 
     Returns:
         sagemaker.fw_utils.UserCode: An object with the S3 bucket and key (S3 prefix) and script name.
     """
-    lib_dirs = lib_dirs or []
+    dependencies = dependencies or []
     key = '%s/sourcedir.tar.gz' % s3_key_prefix
 
     if directory and directory.lower().startswith('s3://'):
@@ -140,7 +140,7 @@ def tar_and_upload_dir(session, bucket, s3_key_prefix, script, directory, lib_di
         tmp = tempfile.mkdtemp()
 
         try:
-            source_files = _list_files_to_compress(script, directory) + lib_dirs
+            source_files = _list_files_to_compress(script, directory) + dependencies
             tar_file = sagemaker.utils.create_tar_file(source_files, os.path.join(tmp, _TAR_SOURCE_FILENAME))
 
             session.resource('s3').Object(bucket, key).upload_file(tar_file)
diff --git a/src/sagemaker/model.py b/src/sagemaker/model.py
@@ -16,10 +16,10 @@
 
 import sagemaker
 
-from sagemaker.local import LocalSession
-from sagemaker.fw_utils import tar_and_upload_dir, parse_s3_url, model_code_key_prefix
-from sagemaker.session import Session
-from sagemaker.utils import name_from_image, get_config_value
+from sagemaker import local
+from sagemaker import fw_utils
+from sagemaker import session
+from sagemaker import utils
 
 
 class Model(object):
@@ -96,12 +96,12 @@ def deploy(self, initial_instance_count, instance_type, endpoint_name=None, tags
         """
         if not self.sagemaker_session:
             if instance_type in ('local', 'local_gpu'):
-                self.sagemaker_session = LocalSession()
+                self.sagemaker_session = local.LocalSession()
             else:
-                self.sagemaker_session = Session()
+                self.sagemaker_session = session.Session()
 
         container_def = self.prepare_container_def(instance_type)
-        self.name = self.name or name_from_image(container_def['Image'])
+        self.name = self.name or utils.name_from_image(container_def['Image'])
         self.sagemaker_session.create_model(self.name, self.role, container_def, vpc_config=self.vpc_config)
         production_variant = sagemaker.production_variant(self.name, instance_type, initial_instance_count)
         self.endpoint_name = endpoint_name or self.name
@@ -127,7 +127,7 @@ class FrameworkModel(Model):
 
     def __init__(self, model_data, image, role, entry_point, source_dir=None, predictor_cls=None, env=None, name=None,
                  enable_cloudwatch_metrics=False, container_log_level=logging.INFO, code_location=None,
-                 sagemaker_session=None, lib_dirs=None, **kwargs):
+                 sagemaker_session=None, dependencies=None, **kwargs):
         """Initialize a ``FrameworkModel``.
 
         Args:
@@ -140,14 +140,14 @@ def __init__(self, model_data, image, role, entry_point, source_dir=None, predic
                 source code dependencies aside from tne entry point file (default: None). Structure within this
                 directory will be preserved when training on SageMaker.
                 If the directory points to S3, no code will be uploaded and the S3 location will be used instead.
-            lib_dirs (list[str]): A list of paths to directories (absolute or relative) with
+            dependencies (list[str]): A list of paths to directories (absolute or relative) with
                 any additional libraries that will be exported to the container (default: []).
                 The library folders will be copied to SageMaker in the same folder where the entrypoint is copied.
                 If the ```source_dir``` points to S3, code will be uploaded and the S3 location will be used
                 instead. Example:
 
                     The following call
-                    >>> Estimator(entry_point='train.py', lib_dirs=['my/libs/common', 'virtual-env'])
+                    >>> Estimator(entry_point='train.py', dependencies=['my/libs/common', 'virtual-env'])
                     results in the following inside the container:
 
                     >>> $ ls
@@ -177,11 +177,11 @@ def __init__(self, model_data, image, role, entry_point, source_dir=None, predic
                                              sagemaker_session=sagemaker_session, **kwargs)
         self.entry_point = entry_point
         self.source_dir = source_dir
-        self.lib_dirs = lib_dirs or []
+        self.dependencies = dependencies or []
         self.enable_cloudwatch_metrics = enable_cloudwatch_metrics
         self.container_log_level = container_log_level
         if code_location:
-            self.bucket, self.key_prefix = parse_s3_url(code_location)
+            self.bucket, self.key_prefix = fw_utils.parse_s3_url(code_location)
         else:
             self.bucket, self.key_prefix = None, None
         self.uploaded_code = None
@@ -197,23 +197,24 @@ def prepare_container_def(self, instance_type):  # pylint disable=unused-argumen
         Returns:
             dict[str, str]: A container definition object usable with the CreateModel API.
         """
-        deploy_key_prefix = model_code_key_prefix(self.key_prefix, self.name, self.image)
+        deploy_key_prefix = fw_utils.model_code_key_prefix(self.key_prefix, self.name, self.image)
         self._upload_code(deploy_key_prefix)
         deploy_env = dict(self.env)
         deploy_env.update(self._framework_env_vars())
         return sagemaker.container_def(self.image, self.model_data, deploy_env)
 
     def _upload_code(self, key_prefix):
-        local_code = get_config_value('local.local_code', self.sagemaker_session.config)
+        local_code = utils.get_config_value('local.local_code', self.sagemaker_session.config)
         if self.sagemaker_session.local_mode and local_code:
             self.uploaded_code = None
         else:
-            self.uploaded_code = tar_and_upload_dir(session=self.sagemaker_session.boto_session,
-                                                    bucket=self.bucket or self.sagemaker_session.default_bucket(),
-                                                    s3_key_prefix=key_prefix,
-                                                    script=self.entry_point,
-                                                    directory=self.source_dir,
-                                                    lib_dirs=self.lib_dirs)
+            bucket = self.bucket or self.sagemaker_session.default_bucket()
+            self.uploaded_code = fw_utils.tar_and_upload_dir(session=self.sagemaker_session.boto_session,
+                                                             bucket=bucket,
+                                                             s3_key_prefix=key_prefix,
+                                                             script=self.entry_point,
+                                                             directory=self.source_dir,
+                                                             dependencies=self.dependencies)
 
     def _framework_env_vars(self):
         if self.uploaded_code:
diff --git a/src/sagemaker/mxnet/README.rst b/src/sagemaker/mxnet/README.rst
@@ -271,14 +271,14 @@ The following are optional arguments. When you create an ``MXNet`` object, you c
    other training source code dependencies including the entry point
    file. Structure within this directory will be preserved when training
    on SageMaker.
-- ``lib_dirs (list[str])`` A list of paths to directories (absolute or relative) with
+- ``dependencies (list[str])`` A list of paths to directories (absolute or relative) with
         any additional libraries that will be exported to the container (default: []).
         The library folders will be copied to SageMaker in the same folder where the entrypoint is copied.
         If the ```source_dir``` points to S3, code will be uploaded and the S3 location will be used
         instead. Example:
 
             The following call
-            >>> Estimator(entry_point='train.py', lib_dirs=['my/libs/common', 'virtual-env'])
+            >>> MXNet(entry_point='train.py', dependencies=['my/libs/common', 'virtual-env'])
             results in the following inside the container:
 
             >>> $ ls
diff --git a/src/sagemaker/mxnet/estimator.py b/src/sagemaker/mxnet/estimator.py
@@ -115,7 +115,7 @@ def create_model(self, model_server_workers=None, role=None, vpc_config_override
                           container_log_level=self.container_log_level, code_location=self.code_location,
                           py_version=self.py_version, framework_version=self.framework_version, image=self.image_name,
                           model_server_workers=model_server_workers, sagemaker_session=self.sagemaker_session,
-                          vpc_config=self.get_vpc_config(vpc_config_override), lib_dirs=self.lib_dirs)
+                          vpc_config=self.get_vpc_config(vpc_config_override), dependencies=self.dependencies)
 
     @classmethod
     def _prepare_init_params_from_job_description(cls, job_details, model_channel_name=None):
diff --git a/src/sagemaker/pytorch/README.rst b/src/sagemaker/pytorch/README.rst
@@ -175,14 +175,14 @@ The following are optional arguments. When you create a ``PyTorch`` object, you
    other training source code dependencies including the entry point
    file. Structure within this directory will be preserved when training
    on SageMaker.
-- ``lib_dirs (list[str])`` A list of paths to directories (absolute or relative) with
+- ``dependencies (list[str])`` A list of paths to directories (absolute or relative) with
         any additional libraries that will be exported to the container (default: []).
         The library folders will be copied to SageMaker in the same folder where the entrypoint is copied.
         If the ```source_dir``` points to S3, code will be uploaded and the S3 location will be used
         instead. Example:
 
             The following call
-            >>> Estimator(entry_point='train.py', lib_dirs=['my/libs/common', 'virtual-env'])
+            >>> PyTorch(entry_point='train.py', dependencies=['my/libs/common', 'virtual-env'])
             results in the following inside the container:
 
             >>> $ ls
diff --git a/src/sagemaker/pytorch/estimator.py b/src/sagemaker/pytorch/estimator.py
@@ -96,7 +96,7 @@ def create_model(self, model_server_workers=None, role=None, vpc_config_override
                             container_log_level=self.container_log_level, code_location=self.code_location,
                             py_version=self.py_version, framework_version=self.framework_version, image=self.image_name,
                             model_server_workers=model_server_workers, sagemaker_session=self.sagemaker_session,
-                            vpc_config=self.get_vpc_config(vpc_config_override), lib_dirs=self.lib_dirs)
+                            vpc_config=self.get_vpc_config(vpc_config_override), dependencies=self.dependencies)
 
     @classmethod
     def _prepare_init_params_from_job_description(cls, job_details, model_channel_name=None):
diff --git a/src/sagemaker/tensorflow/README.rst b/src/sagemaker/tensorflow/README.rst
@@ -409,14 +409,14 @@ you can specify these as keyword arguments.
    other training source code dependencies including the entry point
    file. Structure within this directory will be preserved when training
    on SageMaker.
-- ``lib_dirs (list[str])`` A list of paths to directories (absolute or relative) with
+- ``dependencies (list[str])`` A list of paths to directories (absolute or relative) with
         any additional libraries that will be exported to the container (default: []).
         The library folders will be copied to SageMaker in the same folder where the entrypoint is copied.
         If the ```source_dir``` points to S3, code will be uploaded and the S3 location will be used
         instead. Example:
 
             The following call
-            >>> Estimator(entry_point='train.py', lib_dirs=['my/libs/common', 'virtual-env'])
+            >>> TensorFlow(entry_point='train.py', dependencies=['my/libs/common', 'virtual-env'])
             results in the following inside the container:
 
             >>> $ ls
diff --git a/src/sagemaker/tensorflow/estimator.py b/src/sagemaker/tensorflow/estimator.py
@@ -412,7 +412,7 @@ def _create_default_model(self, model_server_workers, role, vpc_config_override)
                                model_server_workers=model_server_workers,
                                sagemaker_session=self.sagemaker_session,
                                vpc_config=self.get_vpc_config(vpc_config_override),
-                               lib_dirs=self.lib_dirs)
+                               dependencies=self.dependencies)
 
     def hyperparameters(self):
         """Return hyperparameters used by your custom TensorFlow code during model training."""
diff --git a/tests/data/pytorch_source_dirs/train.py b/tests/data/pytorch_source_dirs/train.py
@@ -10,31 +10,21 @@
 # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
 # ANY KIND, either express or implied. See the License for the specific
 # language governing permissions and limitations under the License.
-# import os
-# import sys
-# import tarfile
-#
-# lib_dir = '/opt/ml/lib'
-#
-# if not os.path.exists(lib_dir):
-#     os.makedirs(lib_dir)
-#
-# with tarfile.open(name=os.path.join(os.path.dirname(__file__), 'opt_ml_lib.tar.gz'), mode='r:gz') as t:
-#     t.extractall(path=lib_dir)
-#
-# sys.path.insert(0, lib_dir)
-
 import alexa
+import json
+
+MODEL = '/opt/ml/model/answer'
 
 
 def model_fn(anything):
-    return alexa
+    with open(MODEL) as model:
+        return json.load(model)
 
 
 def predict_fn(input_object, model):
-    return input_object
+    return input_object + model
 
 
 if __name__ == '__main__':
-    with open('/opt/ml/model/answer', 'w') as model:
-        model.write(str(alexa.question('How many roads must a man walk down?')))
+    with open(MODEL, 'w') as model:
+        json.dump(alexa.question('How many roads must a man walk down?'), model)
diff --git a/tests/integ/test_source_dirs.py b/tests/integ/test_source_dirs.py
@@ -25,16 +25,16 @@ def test_source_dirs(sagemaker_session, tmpdir):
     with open(lib, 'w') as f:
         f.write('def question(to_anything): return 42')
 
-    estimator = PyTorch(entry_point='train.py', role='SageMakerRole', source_dir=source_dir, lib_dirs=[lib],
+    estimator = PyTorch(entry_point='train.py', role='SageMakerRole', source_dir=source_dir, dependencies=[lib],
                         py_version=PYTHON_VERSION, train_instance_count=1, train_instance_type='local')
     try:
 
         estimator.fit()
 
         predictor = estimator.deploy(initial_instance_count=1, instance_type='local')
 
-        predict_response = predictor.predict([24])
+        predict_response = predictor.predict([7])
 
-        assert predict_response == [24]
+        assert predict_response == [49]
     finally:
         estimator.delete_endpoint()
diff --git a/tests/unit/test_fw_utils.py b/tests/unit/test_fw_utils.py
diff --git a/tests/unit/test_model.py b/tests/unit/test_model.py
