Skip to content

Commit 7583222

Browse files
mohamedzeidan2021Mohamed Zeidan
andauthored
feat: allow configurable S3 key prefix for ModelTrainer input uploads (#5638) (#6354)
Add an optional ModelTrainer.input_s3_key_prefix field. When set, it is used as the leading segment of the input-data S3 key prefix (e.g. a pipeline name) instead of base_job_name; when None, the upload layout is unchanged. Covers the SDK-managed code/sm_drivers/recipe channels and local input channels via _create_training_job_args, and the standalone create_input_data_channel default branch. Co-authored-by: Mohamed Zeidan <zeidmo@amazon.com>
1 parent 200447e commit 7583222

2 files changed

Lines changed: 95 additions & 2 deletions

File tree

‎sagemaker-train/src/sagemaker/train/model_trainer.py‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,12 @@ class ModelTrainer(BaseModel):
206206
The base name for the training job.
207207
If not specified, a default name will be generated using the algorithm name
208208
or training image name.
209+
input_s3_key_prefix (Optional[str]):
210+
The leading S3 key prefix under which locally-staged input data (source code,
211+
SDK drivers, and local input channels) is uploaded. This is useful for grouping
212+
a training job's artifacts under a custom prefix such as a pipeline name, e.g.
213+
``s3://<default_bucket_path>/<input_s3_key_prefix>/<job_name>/input/``. If not
214+
specified, ``base_job_name`` is used as the leading prefix (unchanged behavior).
209215
source_code (Optional[SourceCode]):
210216
The source code configuration. This is used to configure the source code for
211217
running the training job.
@@ -274,6 +280,7 @@ class ModelTrainer(BaseModel):
274280
sagemaker_session: Optional[Session] = None
275281
role: Optional[str] = None
276282
base_job_name: Optional[str] = None
283+
input_s3_key_prefix: Optional[str] = None
277284
source_code: Optional[SourceCode] = None
278285
distributed: Optional[DistributedConfig] = None
279286
compute: Optional[Compute] = None
@@ -317,6 +324,7 @@ class ModelTrainer(BaseModel):
317324
CONFIGURABLE_ATTRIBUTES: ClassVar[List[str]] = [
318325
"role",
319326
"base_job_name",
327+
"input_s3_key_prefix",
320328
"source_code",
321329
"compute",
322330
"networking",
@@ -620,7 +628,10 @@ def _create_training_job_args(
620628
"""
621629
self._populate_intelligent_defaults()
622630
current_training_job_name = _get_unique_name(self.base_job_name)
623-
input_data_key_prefix = f"{self.base_job_name}/{current_training_job_name}/input"
631+
# Use input_s3_key_prefix (e.g. a pipeline name) as the leading key prefix when set;
632+
# otherwise fall back to base_job_name to preserve the default upload layout.
633+
input_data_key_prefix_root = self.input_s3_key_prefix or self.base_job_name
634+
input_data_key_prefix = f"{input_data_key_prefix_root}/{current_training_job_name}/input"
624635

625636
final_input_data_config = self.input_data_config.copy() if self.input_data_config else []
626637

@@ -1122,7 +1133,8 @@ def create_input_data_channel(
11221133
key_prefix = (
11231134
f"{key_prefix}/{channel_name}"
11241135
if key_prefix
1125-
else f"{self.base_job_name}/input/{channel_name}"
1136+
else f"{self.input_s3_key_prefix or self.base_job_name}"
1137+
f"/input/{channel_name}"
11261138
)
11271139
if self.sagemaker_session.default_bucket_prefix:
11281140
key_prefix = f"{self.sagemaker_session.default_bucket_prefix}/{key_prefix}"

‎sagemaker-train/tests/unit/train/test_model_trainer.py‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,87 @@ def test_create_input_data_channel_with_instance_group_names(model_trainer):
582582
]
583583

584584

585+
@patch("sagemaker.train.model_trainer.ModelTrainer._resolve_staging_bucket")
586+
@patch("sagemaker.train.model_trainer.Session.upload_data")
587+
@patch("sagemaker.train.model_trainer.Session.default_bucket")
588+
def test_create_input_data_channel_default_prefix_uses_base_job_name(
589+
mock_default_bucket, mock_upload_data, mock_staging, model_trainer
590+
):
591+
"""Without input_s3_key_prefix, the upload key prefix is derived from base_job_name (unchanged)."""
592+
mock_default_bucket.return_value = DEFAULT_BUCKET
593+
mock_staging.return_value = (DEFAULT_BUCKET, None)
594+
mock_upload_data.return_value = f"s3://{DEFAULT_BUCKET}/code"
595+
596+
assert model_trainer.input_s3_key_prefix is None
597+
model_trainer.create_input_data_channel("code", DEFAULT_SOURCE_DIR)
598+
599+
# Leading segment is derived from base_job_name when input_s3_key_prefix is unset.
600+
assert mock_upload_data.call_args.kwargs["key_prefix"] == f"{DEFAULT_BASE_NAME}/input/code"
601+
602+
603+
@patch("sagemaker.train.model_trainer.ModelTrainer._resolve_staging_bucket")
604+
@patch("sagemaker.train.model_trainer.Session.upload_data")
605+
@patch("sagemaker.train.model_trainer.Session.default_bucket")
606+
def test_create_input_data_channel_custom_input_s3_key_prefix(
607+
mock_default_bucket, mock_upload_data, mock_staging
608+
):
609+
"""input_s3_key_prefix replaces base_job_name as the leading key prefix (issue #5638)."""
610+
mock_default_bucket.return_value = DEFAULT_BUCKET
611+
mock_staging.return_value = (DEFAULT_BUCKET, None)
612+
mock_upload_data.return_value = f"s3://{DEFAULT_BUCKET}/code"
613+
614+
trainer = ModelTrainer(
615+
training_image=DEFAULT_IMAGE,
616+
role=DEFAULT_ROLE,
617+
compute=DEFAULT_COMPUTE_CONFIG,
618+
stopping_condition=DEFAULT_STOPPING_CONDITION,
619+
output_data_config=DEFAULT_OUTPUT_DATA_CONFIG,
620+
input_s3_key_prefix="my-pipeline/my-step",
621+
)
622+
trainer.create_input_data_channel("code", DEFAULT_SOURCE_DIR)
623+
624+
# input_s3_key_prefix replaces base_job_name as the leading segment.
625+
assert mock_upload_data.call_args.kwargs["key_prefix"] == "my-pipeline/my-step/input/code"
626+
assert f"{DEFAULT_BASE_NAME}/input/code" not in mock_upload_data.call_args.kwargs["key_prefix"]
627+
628+
629+
@patch("sagemaker.train.model_trainer.ModelTrainer._resolve_staging_bucket")
630+
@patch("sagemaker.train.model_trainer.Session.upload_data")
631+
@patch("sagemaker.train.model_trainer.Session.default_bucket")
632+
def test_create_training_job_args_input_s3_key_prefix(
633+
mock_default_bucket, mock_upload_data, mock_staging
634+
):
635+
"""Managed sm_drivers/code channels use input_s3_key_prefix in their S3 URIs (issue #5638)."""
636+
mock_default_bucket.return_value = DEFAULT_BUCKET
637+
mock_staging.return_value = (DEFAULT_BUCKET, None)
638+
639+
def _echo(path, bucket, key_prefix, extra_args=None):
640+
return f"s3://{bucket}/{key_prefix}"
641+
642+
mock_upload_data.side_effect = _echo
643+
644+
trainer = ModelTrainer(
645+
training_image=DEFAULT_IMAGE,
646+
role=DEFAULT_ROLE,
647+
source_code=DEFAULT_SOURCE_CODE,
648+
compute=DEFAULT_COMPUTE_CONFIG,
649+
stopping_condition=DEFAULT_STOPPING_CONDITION,
650+
output_data_config=DEFAULT_OUTPUT_DATA_CONFIG,
651+
input_s3_key_prefix="my-pipeline",
652+
)
653+
args = trainer._create_training_job_args()
654+
uris = {
655+
channel.channel_name: channel.data_source.s3_data_source.s3_uri
656+
for channel in args["input_data_config"]
657+
if channel.data_source and channel.data_source.s3_data_source
658+
}
659+
660+
# The SDK-managed sm_drivers channel is staged from a local temp dir, so its S3 URI
661+
# is built from the input key prefix -> it must lead with the custom pipeline prefix.
662+
assert "/my-pipeline/" in uris["sm_drivers"]
663+
assert f"/{DEFAULT_BASE_NAME}/input" not in uris["sm_drivers"]
664+
665+
585666
HETEROGENEOUS_INSTANCE_GROUPS = [
586667
InstanceGroup(
587668
instance_type="ml.t3.large", instance_count=1, instance_group_name="head-instance-group"

0 commit comments

Comments
 (0)