-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feature: Adding serial inference pipeline support to RegisterModel Step #2405
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
Changes from 6 commits
0a0ce3c
5e90186
6561856
a4a352a
f23ff20
f52ab31
9528898
e86f8dd
cd85320
5f2c6c1
deda665
bcf55af
562b56e
ee2b1bc
9c4a914
bdf809f
ced6d1c
bb5fd97
151d3bb
06f9659
1a349c3
aaf4d52
382da53
a5b1c43
36287f5
faac0a7
a0f446f
2b7f86b
f84c50f
a614e6e
a0abe80
ef1b929
6713ff6
6538f6b
5261a2f
d95235f
80e391e
6eeff17
89ed8e2
22fccc0
6e12e41
c2d5e61
4a8672e
33db77f
a752214
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -205,18 +205,19 @@ class _RegisterModelStep(Step): | |
Estimator's training container image will be used (default: None). | ||
compile_model_family (str): Instance family for compiled model, if specified, a compiled | ||
model will be used (default: None). | ||
container_def_list (list): A list of container defintiions. | ||
**kwargs: additional arguments to `create_model`. | ||
""" | ||
|
||
def __init__( | ||
self, | ||
name: str, | ||
estimator: EstimatorBase, | ||
model_data, | ||
content_types, | ||
response_types, | ||
inference_instances, | ||
transform_instances, | ||
model_data=None, | ||
model_package_group_name=None, | ||
model_metrics=None, | ||
metadata_properties=None, | ||
|
@@ -225,6 +226,7 @@ def __init__( | |
compile_model_family=None, | ||
description=None, | ||
depends_on: List[str] = None, | ||
container_def_list=None, | ||
**kwargs, | ||
): | ||
"""Constructor of a register model step. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit - see below that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure i shall update. |
||
|
@@ -254,6 +256,7 @@ def __init__( | |
description (str): Model Package description (default: None). | ||
depends_on (List[str]): A list of step names this `sagemaker.workflow.steps.TrainingStep` | ||
depends on | ||
container_def_list (list): A list of container defintiions. | ||
**kwargs: additional arguments to `create_model`. | ||
""" | ||
super(_RegisterModelStep, self).__init__(name, StepTypeEnum.REGISTER_MODEL, depends_on) | ||
|
@@ -270,6 +273,7 @@ def __init__( | |
self.image_uri = image_uri | ||
self.compile_model_family = compile_model_family | ||
self.description = description | ||
self.container_def_list = container_def_list | ||
self.kwargs = kwargs | ||
|
||
self._properties = Properties( | ||
|
@@ -301,7 +305,7 @@ def arguments(self) -> RequestType: | |
self.estimator.output_path = output_path | ||
|
||
# yeah, there is some framework stuff going on that we need to pull in here | ||
if model.image_uri is None: | ||
if model.image_uri is None and self.container_def_list is None: | ||
region_name = self.estimator.sagemaker_session.boto_session.region_name | ||
model.image_uri = image_uris.retrieve( | ||
model._framework_name, | ||
|
@@ -324,6 +328,7 @@ def arguments(self) -> RequestType: | |
metadata_properties=self.metadata_properties, | ||
approval_status=self.approval_status, | ||
description=self.description, | ||
container_def_list=self.container_def_list, | ||
) | ||
request_dict = model.sagemaker_session._get_create_model_package_request( | ||
**model_package_args | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this change necessary?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, now that we are passing model list, this input may be empty, hence marked it as None.