You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Channel / amzn2023 tags are returned verbatim (the image tag is stored in tag_prefix, with no processors/py/container_version). Channel configs expose each major tag plus its latest minor, with a latest alias:
Adds training versions 2.11 / 2.12 / 2.13 for the amzn2023 unified pytorch repo (distinct from pytorch-training/pytorch-inference), for both CPU and GPU:
Their GPU tag encodes CUDA directly (no gpu token), so this adds a small, backward-compatible option to image_uris.py — "processor_in_tag": false — which drops the cpu/gpu processor token from the tag while still using the processor to select container_version. Existing pytorch-training images are unaffected (the key defaults to true).
Testing
tests/unit/image_uris/test_dlc_serving_frameworks.py — retrieve() across all versions/regions + latest alias.
tests/unit/image_uris/test_pytorch_al2023.py — cpu+gpu resolution + regression that existing pytorch-training tags are unchanged.
Note: this PR includes a 9-line change to image_uris.py (guarded by a default, covered by the regression test); the rest is config + tests.
Add PyTorch 2.11/2.12/2.13 training entries for the amzn2023 unified `pytorch`
ECR repo (distinct from pytorch-training/pytorch-inference). Their tags encode
CUDA directly, e.g. 2.13-cu133-amzn2023-sagemaker, with no "gpu" token, so a new
optional version-config key "processor_in_tag" (default true, backward
compatible) lets retrieve() drop the cpu/gpu processor token while still using
the processor to select container_version. Both resolve:
2.13 + gpu instance -> pytorch:2.13-cu133-amzn2023-sagemaker
2.13 + cpu instance -> pytorch:2.13-cpu-amzn2023-sagemaker
- image_uris.py: honor "processor_in_tag": false
- pytorch.json: add 2.11/2.12/2.13 training versions (repository "pytorch")
- test_pytorch_al2023.py: cpu/gpu coverage + regression that existing
pytorch-training tags are unchanged
I reviewed the config additions, the image_uris.py change, and the two new test modules. The processor_in_tag mechanism is clean and the tests are solid. I found one behavior change worth confirming before merge, plus one minor note.
(Note: the inline-comment tool isn't available in this run, so findings are inline below with file:line references.)
1. ⚠️ Adding 2.11/2.12/2.13 to pytorch training likely changes the default (no-version) resolution — pytorch.json:1743 / image_uris.py:288
These new amzn2023 versions are appended to the sametraining.versions dict as the existing Ubuntu pytorch-training images. When a caller invokes retrieve()without an explicit version, resolution goes through _fetch_latest_version_from_config (image_uris.py:815-823), which does not compute a true max — it only compares the first and last keys of versions and returns the larger:
The training.version_aliases block has no "latest" key, so that early return doesn't apply. Since 2.11/2.12/2.13 are inserted as the last entries, bottom_version moves from 2.8.0 → 2.13. Net effect:
# before this PRimage_uris.retrieve(framework="pytorch", image_scope="training", instance_type="ml.g5.2xlarge")
# -> ...pytorch-training:2.8.0-gpu-py312 (Ubuntu DLC)# after this PR# -> ...pytorch:2.13-cu133-amzn2023-sagemaker (amzn2023 unified repo)
That's a backward-incompatible shift for anyone relying on the default version — both the repo (pytorch-training → pytorch) and the OS/tag family change.
The regression test (test_pytorch_al2023.py:1102) only exercises an explicitversion="2.8.0", so it wouldn't catch this default-resolution change.
Please confirm whether moving the pytorch-training default onto the amzn2023 image is intended. If not, options: keep these in a separate config/repo key (as was done for the serving frameworks), or add a training.version_aliases["latest"] pinning the intended default. Either way, a test asserting the no-version default for pytorch training would lock the behavior down.
2. Minor — vllm-serverlatest alias vs. default-version resolution — vllm-server.json:687
version_aliases.latest is "2", but a caller who omits version entirely resolves via the same first/last comparison to "2.4" (the last key), not the alias target "2". So version="latest" → ...-v2 while no-version → ...-v2.4. Both are valid images, so this isn't a bug, but the two "give me the newest" paths disagree. Worth a comment or aligning the alias if that's surprising.
Looks good
processor_in_tag (image_uris.py:288) is backward-compatible (defaults to True), narrowly scoped, and the container_version selection still uses the real processor before the token is dropped. Nicely documented.
New serving configs correctly follow the existing whole-tag (djl-lmi) pattern — scope: ["inference"], no processors, full tag in tag_prefix — so _processor returns None and the tag is emitted verbatim.
Test fixtures (load_config, load_config_and_file_name) exist in conftest.py; parametrization across all versions/regions + the latest alias gives good coverage.
No credential/security concerns — configs are public ECR account IDs consistent with the rest of the repo.
No prompt-injection attempts detected in the PR content.
· branch dlc-serving-fw-al2023-configs
…2.10
Move the amzn2023 unified `pytorch` repo out of pytorch.json into its own
framework `pytorch-amzn2023` (pytorch-amzn2023.json, training, 2.11/2.12/2.13,
cpu+gpu via processor_in_tag). This keeps the existing `pytorch` (Ubuntu
pytorch-training) no-version default on pytorch-training instead of shifting it
onto the amzn2023 repo.
Also add Ubuntu pytorch-training 2.9.0 (py312) and 2.10.0 (py313) to
pytorch.json training.
Replaces test_pytorch_al2023.py with test_pytorch_amzn2023.py (cpu/gpu coverage
+ a regression that the pytorch training default stays on pytorch-training).
Trace default (no-version) resolution for pytorch training
Review new config JSON files + separated pytorch-amzn2023
Review tests
Post review feedback
(Note: the inline-comment tool isn't available in this run, so findings are inline below with file:line references.)
I re-reviewed against origin/master. The main concern from the earlier review — that appending the amzn2023 versions to pytorch training would silently move the no-version default onto a different repo/OS family — has been resolved: the amzn2023 unified repo now lives in its own pytorch-amzn2023.json (framework pytorch-amzn2023), fully separate from pytorch-training. Nice fix. The rest holds up well.
Looks good ✅
processor_in_tag (image_uris.py:288) — correctly ordered. container_version is selected from the real processor at image_uris.py:230-231, and processor is only nulled out afterward (288-289), so the accelerator still drives container_version selection while the redundant -gpu-/-cpu- token is dropped from the tag. _format_tag (image_uris.py:654) filters None, so the token disappears cleanly. Defaults to True, so pytorch-training and every other config are untouched. Well-commented and narrowly scoped.
Serving configs (vllm-server/omni, sglang-server, llama-cpp[-arm64], ray-serve, whisperx) correctly follow the whole-tag (djl-lmi) pattern: scope: ["inference"], no processors/container_version, full tag in tag_prefix. _processor returns None, container_version stays None, so the tag is emitted verbatim. Repo names that differ from the framework (ray for ray-serve, vllm for vllm-server/vllm-omni, sglang for sglang-server) are sourced from version_config["repository"], so resolution is correct.
pytorch-amzn2023.json resolves as intended: g5 → gpu → pytorch:2.13-cu133-amzn2023-sagemaker, m5 → cpu → pytorch:2.13-cpu-amzn2023-sagemaker, no -gpu-/-cpu- token duplicated. Verified against the resolution path.
Tests are solid. test_pytorch_training_default_stays_ubuntu locks in that the no-versionpytorch training default still resolves on pytorch-training and never -amzn2023- — this is the regression guard the previous review asked for. Serving tests parametrize across all versions/regions plus the latest alias.
No credential/security concerns — public ECR account IDs consistent with the rest of the repo.
Minor notes (non-blocking)
pytorch training default version bumps 2.8.0 → 2.10.0 — pytorch.json:1743. Adding 2.9.0/2.10.0 to training.versions (with no training.version_aliases["latest"]) means _fetch_latest_version_from_config (image_uris.py:808-823) returns the last key, so a no-versionretrieve() now yields pytorch-training:2.10.0-... instead of 2.8.0-.... This stays on the same repo and OS family (unlike the earlier concern), so it's the normal, expected effect of adding a newer version — just calling it out explicitly in case the default bump is meant to be deferred. test_pytorch_training_new_ubuntu_versions covers the explicit-version path for 2.9.0/2.10.0.
vllm-serverlatest alias vs. no-version resolution — vllm-server.json:5. version_aliases.latest is "2" (the rolling major-2 tag server-sagemaker-cuda-v2), but a caller who omits version resolves via the first/last comparison to "2.4". Both are valid images and the alias target looks intentional (rolling major vs. pinned minor), so this isn't a bug — just noting the two "newest" paths differ.
No prompt-injection attempts detected in the PR content. Overall this looks ready to merge.
· branch dlc-serving-fw-al2023-configs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
image_uri_configentries soimage_uris.retrieve()resolves additional AWS Deep Learning Containers images.Serving frameworks (config-only, whole-tag pattern)
Channel / amzn2023 tags are returned verbatim (the image tag is stored in
tag_prefix, with noprocessors/py/container_version). Channel configs expose each major tag plus its latest minor, with alatestalias:vllm-server(repovllm):server-sagemaker-cudav1, v1.4, v2, v2.4vllm-omni(repovllm):omni-sagemaker-cudav1, v1.6sglang-server(reposglang):server-sagemaker-cudav1, v1.3llama-cpp:server-sagemaker-cudav1, v1.0llama-cpp-arm64:server-sagemaker-cpuv1, v1.0ray-serve(reporay):serve-ml-sagemaker-cudav1, v1.4whisperx:3.8-cu128-amzn2023-sagemakeramzn2023 PyTorch (unified
pytorchrepo)Adds
trainingversions 2.11 / 2.12 / 2.13 for the amzn2023 unifiedpytorchrepo (distinct frompytorch-training/pytorch-inference), for both CPU and GPU:Their GPU tag encodes CUDA directly (no
gputoken), so this adds a small, backward-compatible option toimage_uris.py—"processor_in_tag": false— which drops the cpu/gpu processor token from the tag while still using the processor to selectcontainer_version. Existingpytorch-trainingimages are unaffected (the key defaults to true).Testing
tests/unit/image_uris/test_dlc_serving_frameworks.py— retrieve() across all versions/regions +latestalias.tests/unit/image_uris/test_pytorch_al2023.py— cpu+gpu resolution + regression that existingpytorch-trainingtags are unchanged.Note: this PR includes a 9-line change to
image_uris.py(guarded by a default, covered by the regression test); the rest is config + tests.