-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(sagemaker): add support uncompressed model #30949
base: main
Are you sure you want to change the base?
Changes from 5 commits
5af304a
90cf701
518bfbc
69265e1
a4882a0
88ec55f
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 |
---|---|---|
|
@@ -6,7 +6,58 @@ import { hashcode } from './private/util'; | |
|
||
// The only supported extension for local asset model data | ||
// https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-model-containerdefinition.html#cfn-sagemaker-model-containerdefinition-modeldataurl | ||
const ARTIFACT_EXTENSION = '.tar.gz'; | ||
const COMPRESSED_ARTIFACT_EXTENSION = '.tar.gz'; | ||
|
||
/** | ||
* Specifies how the ML model data is prepared. | ||
*/ | ||
export enum CompressionType { | ||
/** | ||
* If you choose `CompressionType.GZIP` and choose `S3DataType.S3_OBJECT` as the value of `s3DataType`, | ||
* S3 URI identifies an object that is a gzip-compressed TAR archive. | ||
* SageMaker will attempt to decompress and untar the object during model deployment. | ||
*/ | ||
GZIP = 'Gzip', | ||
/** | ||
* If you choose `CompressionType.NONE` and choose `S3DataType.S3_PREFIX` as the value of `s3DataType`, | ||
* S3 URI identifies a key name prefix, under which all objects represents the uncompressed ML model to deploy. | ||
* | ||
* If you choose `CompressionType.NONE`, then SageMaker will follow rules below when creating model data files | ||
* under `/opt/ml/model` directory for use by your inference code: | ||
* - If you choose `S3DataType.S3_OBJECT` as the value of `s3DataType`, then SageMaker will split the key of the S3 object referenced by S3 URI by slash (/), | ||
* and use the last part as the filename of the file holding the content of the S3 object. | ||
* - If you choose `S3DataType.S3_PREFIX` as the value of `s3DataType`, then for each S3 object under the key name pefix referenced by S3 URI, | ||
* SageMaker will trim its key by the prefix, and use the remainder as the path (relative to `/opt/ml/model`) of the file holding the content of the S3 object. | ||
* SageMaker will split the remainder by slash (/), using intermediate parts as directory names and the last part as filename of the file holding the content of the S3 object. | ||
* - Do not use any of the following as file names or directory names: | ||
* - An empty or blank string | ||
* - A string which contains null bytes | ||
* - A string longer than 255 bytes | ||
* - A single dot (.) | ||
* - A double dot (..) | ||
* - Ambiguous file names will result in model deployment failure. | ||
* For example, if your uncompressed ML model consists of two S3 objects `s3://mybucket/model/weights` and `s3://mybucket/model/weights/part1` | ||
* and you specify `s3://mybucket/model/` as the value of S3 URI and `S3DataType.S3_PREFIX` as the value of `s3DataType`, | ||
* then it will result in name clash between `/opt/ml/model/weights` (a regular file) and `/opt/ml/model/weights/` (a directory). | ||
Comment on lines
+32
to
+41
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. This docstring feels unnecessarily lengthy, and the mention of what file/directory names doesn't seem relevant, at least for the enum. Is there anyway we can add a |
||
*/ | ||
NONE = 'None', | ||
} | ||
|
||
/** | ||
* Specifies the type of ML model data to deploy. | ||
*/ | ||
export enum S3DataType { | ||
/** | ||
* If you choose `S3DataType.S3_OBJECT`, S3 UTI identifies an object that is the ML model data to deploy. | ||
*/ | ||
S3_OBJECT = 'S3Object', | ||
/** | ||
* If you choose `S3DataType.S3_PREFIX`, S3 URI identifies a key name prefix. | ||
* SageMaker uses all objects that match the specified key name prefix as part of the ML model data to deploy. | ||
* A valid key name prefix identified by S3 URI always ends with a forward slash (/). | ||
*/ | ||
S3_PREFIX = 'S3Prefix', | ||
} | ||
|
||
/** | ||
* The configuration needed to reference model artifacts. | ||
|
@@ -17,6 +68,16 @@ export interface ModelDataConfig { | |
* must point to a single gzip compressed tar archive (.tar.gz suffix). | ||
*/ | ||
readonly uri: string; | ||
/** | ||
* Specifies how the ML model data is prepared. | ||
* @default CompressionType.GZIP | ||
*/ | ||
readonly compressionType?: CompressionType; | ||
/** | ||
* Specifies the type of ML model data to deploy. | ||
* @default S3DataType.S3_OBJECT | ||
*/ | ||
readonly s3DataType?: S3DataType; | ||
} | ||
|
||
/** | ||
|
@@ -28,9 +89,10 @@ export abstract class ModelData { | |
* Constructs model data which is already available within S3. | ||
* @param bucket The S3 bucket within which the model artifacts are stored | ||
* @param objectKey The S3 object key at which the model artifacts are stored | ||
* @param options The options for identifying model artifacts | ||
*/ | ||
public static fromBucket(bucket: s3.IBucket, objectKey: string): ModelData { | ||
return new S3ModelData(bucket, objectKey); | ||
public static fromBucket(bucket: s3.IBucket, objectKey: string, options?: S3ModelDataOptions): ModelData { | ||
return new S3ModelData(bucket, objectKey, options); | ||
} | ||
|
||
/** | ||
|
@@ -51,8 +113,33 @@ export abstract class ModelData { | |
public abstract bind(scope: Construct, model: IModel): ModelDataConfig; | ||
} | ||
|
||
/** | ||
* The options for identifying model artifacts. | ||
* When you choose `CompressionType.GZIP` and `S3DataType.S3_OBJECT` then use `ModelDataUrl` property. | ||
* Otherwise, use `ModelDataSource` property. | ||
Comment on lines
+118
to
+119
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. Reading this, I interpret it as telling the user to use the |
||
* | ||
* Currently you cannot use ModelDataSource in conjunction with: | ||
* - SageMaker batch transform | ||
* - SageMaker serverless endpoints | ||
* - SageMaker multi-model endpoints | ||
* - SageMaker Marketplace | ||
*/ | ||
export interface S3ModelDataOptions { | ||
/** | ||
* Specifies how the ML model data is prepared. | ||
* @default CompressionType.GZIP | ||
*/ | ||
readonly compressionType: CompressionType; | ||
/** | ||
* Specifies the type of ML model data to deploy. | ||
* @default S3DataType.S3_OBJECT | ||
*/ | ||
readonly s3DataType: S3DataType; | ||
} | ||
|
||
class S3ModelData extends ModelData { | ||
constructor(private readonly bucket: s3.IBucket, private readonly objectKey: string) { | ||
constructor(private readonly bucket: s3.IBucket, | ||
private readonly objectKey: string, private readonly options?: S3ModelDataOptions) { | ||
super(); | ||
} | ||
|
||
|
@@ -61,6 +148,8 @@ class S3ModelData extends ModelData { | |
|
||
return { | ||
uri: this.bucket.urlForObject(this.objectKey), | ||
compressionType: this.options?.compressionType, | ||
s3DataType: this.options?.s3DataType, | ||
}; | ||
} | ||
} | ||
|
@@ -70,9 +159,6 @@ class AssetModelData extends ModelData { | |
|
||
constructor(private readonly path: string, private readonly options: assets.AssetOptions) { | ||
super(); | ||
if (!path.toLowerCase().endsWith(ARTIFACT_EXTENSION)) { | ||
throw new Error(`Asset must be a gzipped tar file with extension ${ARTIFACT_EXTENSION} (${this.path})`); | ||
} | ||
Comment on lines
-73
to
-75
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. The asset's availability determination was moved after binding it as an asset. |
||
} | ||
|
||
public bind(scope: Construct, model: IModel): ModelDataConfig { | ||
|
@@ -83,11 +169,17 @@ class AssetModelData extends ModelData { | |
...this.options, | ||
}); | ||
} | ||
|
||
if (!this.asset.isFile) { | ||
throw new Error(`Asset must be a file, if you want to use directory you can use 'ModelData.fromBucket()' with the 's3DataType' option to 'S3DataType.S3_PREFIX' and 'compressionType' option to 'CompressionType.NONE' (${this.path})`); | ||
} | ||
Comment on lines
+172
to
+174
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. The bundled assets are now allowed if they are not directories. |
||
this.asset.grantRead(model); | ||
|
||
return { | ||
uri: this.asset.httpUrl, | ||
compressionType: this.asset.assetPath.toLowerCase().endsWith(COMPRESSED_ARTIFACT_EXTENSION) | ||
? CompressionType.GZIP | ||
: CompressionType.NONE, | ||
s3DataType: S3DataType.S3_OBJECT, | ||
Comment on lines
+179
to
+182
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. Uncompressed single files are also supported |
||
}; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,10 @@ | ||
import * as ec2 from 'aws-cdk-lib/aws-ec2'; | ||
import * as iam from 'aws-cdk-lib/aws-iam'; | ||
import { CfnModel } from 'aws-cdk-lib/aws-sagemaker'; | ||
import * as cdk from 'aws-cdk-lib/core'; | ||
import { Construct } from 'constructs'; | ||
import { ContainerImage } from './container-image'; | ||
import { ModelData } from './model-data'; | ||
import { CfnModel } from 'aws-cdk-lib/aws-sagemaker'; | ||
import { CompressionType, ModelData, S3DataType } from './model-data'; | ||
|
||
/** | ||
* Interface that defines a Model resource. | ||
|
@@ -357,11 +357,22 @@ export class Model extends ModelBase { | |
} | ||
|
||
private renderContainer(container: ContainerDefinition): CfnModel.ContainerDefinitionProperty { | ||
const image = container.image.bind(this, this); | ||
const modelDataConfig = container.modelData?.bind(this, this); | ||
const useModelDataSource = modelDataConfig?.compressionType === CompressionType.NONE | ||
|| modelDataConfig?.s3DataType === S3DataType.S3_PREFIX; | ||
return { | ||
image: container.image.bind(this, this).imageName, | ||
image: image.imageName, | ||
containerHostname: container.containerHostname, | ||
environment: container.environment, | ||
modelDataUrl: container.modelData ? container.modelData.bind(this, this).uri : undefined, | ||
modelDataSource: useModelDataSource ? { | ||
s3DataSource: { | ||
s3Uri: modelDataConfig.uri, | ||
s3DataType: modelDataConfig.s3DataType!, | ||
compressionType: modelDataConfig.compressionType!, | ||
}, | ||
} : undefined, | ||
modelDataUrl: !useModelDataSource ? modelDataConfig?.uri : undefined, | ||
Comment on lines
+380
to
+387
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. Would it be possible for both of these values to be set at the same time? And if so, what sort of behaviour would we expect? |
||
}; | ||
} | ||
|
||
|
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.
The
CompressionType
andS3DataType
enums seem to be used a lot in conjunction with each other, and when their behaviour changes this drastically between combinations, it makes me wonder if there is any way we can combine the two? Right now, for instance, there doesn't seem to be anything documented about what will happen if I combineCompressionType.GZIP
andS3DataType.S3_PREFIX
, if that's a valid combination.