Skip to content

Model linkage #638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,26 @@
CHANGELOG
=========

1.18.3.dev
1.18.4.dev
==========

* doc-fix: Remove incorrect parameter for EI TFS Python README
* feature: ``Predictor``: delete SageMaker model
* feature: ``Pipeline``: delete SageMaker model

1.18.3.post1
============

* doc-fix: fix README for PyPI

1.18.3
======

* doc-fix: update information about saving models in the MXNet README
* doc-fix: change ReadTheDocs links from latest to stable
* doc-fix: add ``transform_fn`` information and fix ``input_fn`` signature in the MXNet README
* feature: Support for ``Predictor`` to delete endpoint configuration by default when calling ``delete_endpoint()``
* feature: Support for ``model`` to delete SageMaker model
* feature: Support for ``Predictor`` to delete model, and delete endpoint configuration by default when calling ``delete_endpoint()``
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your feature addition needs to be separate and in a new version, as your last PR go merged and released to PyPI already.

https://github.com/aws/sagemaker-python-sdk/releases/tag/v1.18.3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohh. So the next version should be 1.18.4.dev?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say 'add support for..." to be consistent

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even better would be something like "feature: Predictor: delete model and endpoint configuration by default when calling delete_endpoint()", though we've been extremely inconsistent in keeping that format

* feature: Support for ``Model`` to delete SageMaker model
* feature: Support for ``Transformer`` to delete SageMaker model
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we are already not consistent -_- . So it's up to you

* bug-fix: Default account for SKLearnModel fixed

Expand Down
12 changes: 12 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,13 @@ Here is an end to end example of how to use a SageMaker Estimator:
# Tears down the SageMaker endpoint and endpoint configuration
mxnet_predictor.delete_endpoint()

# Deletes SageMaker model
mxnet_predictor.delete_model()

The example above will eventually delete both the SageMaker endpoint and endpoint configuration through `delete_endpoint()`. If you want to keep your SageMaker endpoint configuration, use the value False for the `delete_endpoint_config` parameter, as shown below.

.. code:: python

# Only delete the SageMaker endpoint, while keeping the corresponding endpoint configuration.
mxnet_predictor.delete_endpoint(delete_endpoint_config=False)

Expand Down Expand Up @@ -229,6 +232,9 @@ For more `information <https://boto3.amazonaws.com/v1/documentation/api/latest/r
# Tears down the SageMaker endpoint and endpoint configuration
mxnet_predictor.delete_endpoint()

# Deletes SageMaker model
mxnet_predictor.delete_model()

Training Metrics
~~~~~~~~~~~~~~~~
The SageMaker Python SDK allows you to specify a name and a regular expression for metrics you want to track for training.
Expand Down Expand Up @@ -283,6 +289,9 @@ We can take the example in `Using Estimators <#using-estimators>`__ , and use e
# Tears down the endpoint container and deletes the corresponding endpoint configuration
mxnet_predictor.delete_endpoint()

# Deletes the model
mxnet_predictor.delete_model()


If you have an existing model and want to deploy it locally, don't specify a sagemaker_session argument to the ``MXNetModel`` constructor.
The correct session is generated when you call ``model.deploy()``.
Expand All @@ -306,6 +315,9 @@ Here is an end-to-end example:
# Tear down the endpoint container and delete the corresponding endpoint configuration
predictor.delete_endpoint()

# Deletes the model
predictor.delete_model()


If you don't want to deploy your model locally, you can also choose to perform a Local Batch Transform Job. This is
useful if you want to test your container before creating a Sagemaker Batch Transform Job. Note that the performance
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def read(fname):
required_packages.append('enum34>=1.1.6')

setup(name="sagemaker",
version='1.18.2',
version='1.18.3.post1',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

your branch might be out of date?

description="Open source library for training and deploying models on Amazon SageMaker.",
packages=find_packages('src'),
package_dir={'': 'src'},
Expand Down
11 changes: 11 additions & 0 deletions src/sagemaker/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,14 @@ def deploy(self, initial_instance_count, instance_type, endpoint_name=None, tags
self.sagemaker_session.endpoint_from_production_variants(self.endpoint_name, [production_variant], tags)
if self.predictor_cls:
return self.predictor_cls(self.endpoint_name, self.sagemaker_session)

def delete_model(self):
"""Delete the SageMaker model backing this pipeline model. This does not delete the list of SageMaker models used
in multiple containers to build the inference pipeline.

"""

if self.name is None:
raise ValueError('The SageMaker model must be created before attempting to delete.')

self.sagemaker_session.delete_model(self.name)
32 changes: 26 additions & 6 deletions src/sagemaker/predictor.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ def __init__(self, endpoint, sagemaker_session=None, serializer=None, deserializ
self.deserializer = deserializer
self.content_type = content_type or getattr(serializer, 'content_type', None)
self.accept = accept or getattr(deserializer, 'accept', None)
self._model_names = self._get_model_names()

def predict(self, data, initial_args=None):
"""Return the inference from the specified endpoint.
Expand Down Expand Up @@ -109,23 +110,42 @@ def _delete_endpoint_config(self):
"""Delete the Amazon SageMaker endpoint configuration

"""
endpoint_description = self.sagemaker_session.sagemaker_client.describe_endpoint(EndpointName=self.endpoint)
endpoint_config_name = endpoint_description['EndpointConfigName']
self.sagemaker_session.delete_endpoint_config(endpoint_config_name)
self.sagemaker_session.delete_endpoint_config(self._endpoint_config_name)

def delete_endpoint(self, delete_endpoint_config=True):
"""Delete the Amazon SageMaker endpoint and endpoint configuration backing this predictor.
"""Delete the Amazon SageMaker endpoint backing this predictor. Also delete the endpoint configuration attached
to it if delete_endpoint_config is True.

Args:
delete_endpoint_config (bool): Flag to indicate whether to delete the corresponding SageMaker endpoint
configuration tied to the endpoint. If False, only the endpoint will be deleted. (default: True)
delete_endpoint_config (bool, optional): Flag to indicate whether to delete endpoint configuration together
with endpoint. Defaults to True. If True, both endpoint and endpoint configuration will be deleted. If
False, only endpoint will be deleted.

"""
if delete_endpoint_config:
self._delete_endpoint_config()

self.sagemaker_session.delete_endpoint(self.endpoint)

def delete_model(self):
"""Deletes the Amazon SageMaker models backing this predictor.

"""
for model_name in self._model_names:
self.sagemaker_session.delete_model(model_name)

def _get_endpoint_config_desc(self):
endpoint_desc = self.sagemaker_session.sagemaker_client.describe_endpoint(EndpointName=self.endpoint)
self._endpoint_config_name = endpoint_desc['EndpointConfigName']
endpoint_config = self.sagemaker_session.sagemaker_client.describe_endpoint_config(
EndpointConfigName=self._endpoint_config_name)
return endpoint_config

def _get_model_names(self):
endpoint_config = self._get_endpoint_config_desc()
production_variants = endpoint_config['ProductionVariants']
return map(lambda d: d['ModelName'], production_variants)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it is possible for multiple models to exist behind an endpoint configuration and endpoint.

Let's make sure this class also has the delete_model functionality as well.
https://github.com/aws/sagemaker-python-sdk/blob/master/src/sagemaker/pipeline.py

Here is an example on how it is done:
https://github.com/awslabs/amazon-sagemaker-examples/blob/master/sagemaker-python-sdk/scikit_learn_inference_pipeline/Inference%20Pipeline%20with%20Scikit-learn%20and%20Linear%20Learner.ipynb

We should make sure the deletion of multiple models in the same endpoint configuration is tested in an integ test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something to consider is that this solution assumes the endpoint configuration exists. It is possible for a user to call delete_endpoint() and that might error out when they call delete_model().

Technically, the model object does still exist within SageMaker. It arguably makes sense to actually delete the endpoint first, then endpoint configuration and then model since they all have dependency on each other. I believe SageMaker allows you to delete them regardless of order, since they snapshot everything, however this problem can still persist if the user deletes the endpoint first.

Copy link
Contributor Author

@chuyang-deng chuyang-deng Feb 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm, for Pipeline, delete_model() should delete all the SageMaker models from the list[sagemaker.Model] and delete the model with name (if name is not None)?

And for the order of deletions, if endpoint/endpoint config is deleted before attempting to delete model, should we say something like "model cannot be deleted because the attached endpoint/endpoint config is already gone"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's initialize the model name in the constructor per our in person discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're going to call describe_endpoint() and describe_endpoint_config() once the Predictor gets endpoint_name in the constructor, so that we don't restrict the order of deletions to model -> endpoint/endpoint config.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the existing delete_endpoint() functionality to depend on the endpoint_configuration_name instead of describing it.

endpoint_description = self.sagemaker_session.sagemaker_client.describe_endpoint(EndpointName=self.endpoint)



class _CsvSerializer(object):
def __init__(self):
Expand Down
3 changes: 1 addition & 2 deletions src/sagemaker/tensorflow/deploying_python.rst
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ TensorFlow serving on SageMaker has support for `Elastic Inference <https://docs

predictor = estimator.deploy(initial_instance_count=1,
instance_type='ml.c5.xlarge',
accelerator_type='ml.eia1.medium'
endpoint_type='tensorflow-serving-elastic-inference')
accelerator_type='ml.eia1.medium')

What happens when deploy is called
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
5 changes: 5 additions & 0 deletions tests/integ/test_inference_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,8 @@ def test_inference_pipeline_model_deploy(sagemaker_session):

invalid_data = "1.0,28.0,C,38.0,71.5,1.0"
assert (predictor.predict(invalid_data) is None)

model.delete_model()
with pytest.raises(Exception) as exception:
sagemaker_session.sagemaker_client.describe_model(ModelName=model.name)
assert 'Could not find model' in str(exception.value)
5 changes: 5 additions & 0 deletions tests/integ/test_kmeans.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ def test_kmeans(sagemaker_session):
assert record.label["closest_cluster"] is not None
assert record.label["distance_to_cluster"] is not None

predictor.delete_model()
with pytest.raises(Exception) as exception:
sagemaker_session.sagemaker_client.describe_model(ModelName=model.name)
assert 'Could not find model' in str(exception.value)


def test_async_kmeans(sagemaker_session):
training_job_name = ""
Expand Down
5 changes: 5 additions & 0 deletions tests/integ/test_mxnet_train.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ def test_deploy_model(mxnet_training_job, sagemaker_session, mxnet_full_version)
data = numpy.zeros(shape=(1, 1, 28, 28))
predictor.predict(data)

predictor.delete_model()
with pytest.raises(Exception) as exception:
sagemaker_session.sagemaker_client.describe_model(ModelName=model.name)
assert 'Could not find model' in str(exception.value)


def test_deploy_model_with_update_endpoint(mxnet_training_job, sagemaker_session, mxnet_full_version):
endpoint_name = 'test-mxnet-deploy-model-{}'.format(sagemaker_timestamp())
Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_chainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@
GPU = 'ml.p2.xlarge'
CPU = 'ml.c4.xlarge'

ENDPOINT_DESC = {
'EndpointConfigName': 'test-endpoint'
}

ENDPOINT_CONFIG_DESC = {
'ProductionVariants': [{'ModelName': 'model-1'},
{'ModelName': 'model-2'}]
}


@pytest.fixture()
def sagemaker_session():
Expand All @@ -54,6 +63,8 @@ def sagemaker_session():

describe = {'ModelArtifacts': {'S3ModelArtifacts': 's3://m/m.tar.gz'}}
session.sagemaker_client.describe_training_job = Mock(return_value=describe)
session.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC)
session.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC)
session.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME)
session.expand_role = Mock(name="expand_role", return_value=ROLE)
return session
Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_estimator.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,15 @@
'ModelDataUrl': MODEL_DATA,
}

ENDPOINT_DESC = {
'EndpointConfigName': 'test-endpoint'
}

ENDPOINT_CONFIG_DESC = {
'ProductionVariants': [{'ModelName': 'model-1'},
{'ModelName': 'model-2'}]
}


class DummyFramework(Framework):
__framework_name__ = 'dummy'
Expand Down Expand Up @@ -146,6 +155,8 @@ def sagemaker_session():
sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME)
sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job',
return_value=DESCRIBE_TRAINING_JOB_RESULT)
sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC)
sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC)
return sms


Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_fm.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@
}
}

ENDPOINT_DESC = {
'EndpointConfigName': 'test-endpoint'
}

ENDPOINT_CONFIG_DESC = {
'ProductionVariants': [{'ModelName': 'model-1'},
{'ModelName': 'model-2'}]
}


@pytest.fixture()
def sagemaker_session():
Expand All @@ -47,6 +56,8 @@ def sagemaker_session():
sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME)
sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job',
return_value=DESCRIBE_TRAINING_JOB_RESULT)
sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC)
sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC)
return sms


Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_ipinsights.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@
}
}

ENDPOINT_DESC = {
'EndpointConfigName': 'test-endpoint'
}

ENDPOINT_CONFIG_DESC = {
'ProductionVariants': [{'ModelName': 'model-1'},
{'ModelName': 'model-2'}]
}


@pytest.fixture()
def sagemaker_session():
Expand All @@ -49,6 +58,8 @@ def sagemaker_session():
sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME)
sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job',
return_value=DESCRIBE_TRAINING_JOB_RESULT)
sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC)
sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC)

return sms

Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_kmeans.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@
}
}

ENDPOINT_DESC = {
'EndpointConfigName': 'test-endpoint'
}

ENDPOINT_CONFIG_DESC = {
'ProductionVariants': [{'ModelName': 'model-1'},
{'ModelName': 'model-2'}]
}


@pytest.fixture()
def sagemaker_session():
Expand All @@ -46,6 +55,8 @@ def sagemaker_session():
sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME)
sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job',
return_value=DESCRIBE_TRAINING_JOB_RESULT)
sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC)
sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC)

return sms

Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_knn.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@
}
}

ENDPOINT_DESC = {
'EndpointConfigName': 'test-endpoint'
}

ENDPOINT_CONFIG_DESC = {
'ProductionVariants': [{'ModelName': 'model-1'},
{'ModelName': 'model-2'}]
}


@pytest.fixture()
def sagemaker_session():
Expand All @@ -50,6 +59,8 @@ def sagemaker_session():
sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME)
sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job',
return_value=DESCRIBE_TRAINING_JOB_RESULT)
sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC)
sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC)

return sms

Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_lda.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@
}
}

ENDPOINT_DESC = {
'EndpointConfigName': 'test-endpoint'
}

ENDPOINT_CONFIG_DESC = {
'ProductionVariants': [{'ModelName': 'model-1'},
{'ModelName': 'model-2'}]
}


@pytest.fixture()
def sagemaker_session():
Expand All @@ -44,6 +53,8 @@ def sagemaker_session():
sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME)
sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job',
return_value=DESCRIBE_TRAINING_JOB_RESULT)
sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC)
sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC)

return sms

Expand Down
11 changes: 11 additions & 0 deletions tests/unit/test_linear_learner.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@
}
}

ENDPOINT_DESC = {
'EndpointConfigName': 'test-endpoint'
}

ENDPOINT_CONFIG_DESC = {
'ProductionVariants': [{'ModelName': 'model-1'},
{'ModelName': 'model-2'}]
}


@pytest.fixture()
def sagemaker_session():
Expand All @@ -47,6 +56,8 @@ def sagemaker_session():
sms.default_bucket = Mock(name='default_bucket', return_value=BUCKET_NAME)
sms.sagemaker_client.describe_training_job = Mock(name='describe_training_job',
return_value=DESCRIBE_TRAINING_JOB_RESULT)
sms.sagemaker_client.describe_endpoint = Mock(return_value=ENDPOINT_DESC)
sms.sagemaker_client.describe_endpoint_config = Mock(return_value=ENDPOINT_CONFIG_DESC)

return sms

Expand Down
Loading