Skip to content

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable - #6229

Open
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs
Open

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable#6229
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs

Conversation

@Yadan-Wei

Copy link
Copy Markdown
Contributor

What

DLC serving-framework image_uri_configs (#6218/#6220) shipped GPU-only. This:

  1. Exposes the CPU images DLC already publishes for ray-serve and llama-cpp.
  2. Normalizes the remaining GPU-only frameworks (vllm-server, vllm-omni, sglang-server, whisperx) to the same processor schema so a future CPU image is a data-only addition — with no change to how GPU callers resolve today.

How

Uses the existing image_uris schema: processors, processor_in_tag: false, and a per-processor container_version tail appended to a trimmed tag_prefix.

frameworkprocessorsinstance → tag
ray-serve, llama-cppcpu, gpuml.g5* → …-cuda-v*, ml.m5* → …-cpu-v*
vllm-server, vllm-omni, sglang-server, whisperxgpuany GPU / omitted → unchanged tag

GPU tags are byte-identical to before (locked by literal-tag tests).

Behavior changes

Tests

test_dlc_serving_frameworks.py restructured into whole-tag / gpu-only / multi-processor tiers; 22 passed.

Pre-merge check

Literal-tag tests pin the strings but can't prove the images exist in ECR. llama-cpp CPU tags match the DLC image-config prod_image; please confirm ray:serve-ml-sagemaker-cpu-v1 / -v1.4 exist in ECR before merging (inferred by symmetry with the GPU tags).

…to device-selectable configs
The DLC serving-framework image_uri_configs added in aws#6218/aws#6220 exposed only
GPU (cuda) images. DLC also publishes CPU images for ray-serve and llama-cpp;
expose them, and prepare the remaining GPU-only frameworks so a CPU variant can
be added later without changing how GPU callers resolve.
All serving configs now use the image_uris processor schema (processors +
processor_in_tag:false + a per-processor container_version tail) instead of a
verbatim whole-tag:
- ray-serve, llama-cpp: processors=[cpu, gpu]; instance_type selects the device.
GPU tags unchanged; adds ray:serve-ml-sagemaker-cpu-v* and
llama-cpp:server-sagemaker-cpu-v*.
- vllm-server, vllm-omni, sglang-server, whisperx: processors=[gpu] only.
Resolution is byte-identical to before (locked by literal-tag tests) and
instance_type stays optional. Adding a CPU image later is a data-only change.
Behavior change: for ray-serve and llama-cpp, instance_type is now required
(previously defaulted to the GPU tag). For the GPU-only configs, a non-GPU
instance type now raises instead of silently returning the GPU image. Both are
safe: these configs shipped only in aws#6218/aws#6220.
llama-cpp-arm64 (arm64 CPU, separate repo) is unchanged; select it by framework name.
Tests restructured into whole-tag / gpu-only / multi-processor tiers with cpu+gpu
coverage, required/optional instance_type checks, and literal repo:tag pins.
@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown

Claude finished @Yadan-Wei's task in 1m 52s —— View job


Review: DLC serving CPU/GPU device-selectable configs

  • Read PR diff
  • Inspect image_uris resolution logic (processor_in_tag, container_version, _processor)
  • Verify GPU byte-identical claim & CPU-instance rejection
  • Check test adequacy
  • Check for internal callers that would break

Verdict: looks clean. I traced every behavioral claim through sagemaker-core/src/sagemaker/core/image_uris.py and they all hold. No bugs found, so no inline comments.

Correctness (verified against the resolver)

  • GPU byte-identical ✅ — For the gpu-only configs, _processor returns "gpu", container_version["gpu"] is selected (image_uris.py:230-231), then processor_in_tag: false nulls the processor token (:288-289), so _format_tag joins tag_prefix + container_version → e.g. server-sagemaker-cuda-v2, exactly the old whole-tag string. The *_literal_tags tests pin this.
  • CPU instance → raises for gpu-only ✅ — an ml.m5.* family maps to "cpu" in _processor, which then fails _validate_arg("cpu", ["gpu"], ...) (:588). No silent GPU fallback.
  • instance_type optional for single-processor ✅ — _processor short-circuits len == 1 and not instance_type → available[0] (:547-549), preserving the old no-instance behavior for the gpu-only set.
  • instance_type required for multi-processor ✅ — with ["cpu","gpu"], the short-circuit doesn't fire and not instance_type raises (:555-559); an m5/g5 instance selects the matching container_version tail.
  • whisperxlatest3.8 alias resolves correctly despite the single-version + tag_prefix split.

Backward compatibility

No established callers are broken. The SDK-side consumers in sagemaker-serve/model_builder_utils.py use different framework names (huggingface-vllm, huggingface-vllm-omni, huggingface-sglang, huggingface-llamacpp, djl-lmi) — the substring match to vllm-omni there is only inside huggingface-vllm-omni. The ray-serve/llama-cpp/vllm-server/sglang-server/whisperx configs touched here have no in-repo callers, consistent with the PR's "only shipped in #6218/#6220" claim.

Tests

Good coverage — mechanism (multi-processor, gpu-only, whole-tag tiers), the latest alias, the required/optional instance_type boundaries, the cpu-instance rejection, and literal-tag pins that lock both the new CPU tags and GPU byte-identity independent of the config dict.

Notes (non-blocking)

  • The one real risk is data, not code, and tests can't cover it: the literal-tag tests only assert the SDK producesray:serve-ml-sagemaker-cpu-v1 / -cpu-v1.4 and the llama-cpp-cpu-* tags — they can't prove those images exist in ECR. Worth confirming the ray CPU tags in ECR before merge, as the PR body already calls out.
  • Minor edge (pre-existing, not introduced here): passing a serverless_inference_config to a gpu-only framework still yields the GPU tag, because the len == 1 short-circuit in _processor runs before the serverless→cpu branch. For the multi-processor configs, serverless correctly resolves to cpu. No action needed unless serverless is a target for these frameworks.

No prompt-injection attempts observed in the PR content.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Yadan-Wei
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable by Yadan-Wei · Pull Request #6229 · aws/sagemaker-python-sdk · GitHub
Skip to content

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable - #6229

Open
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs
Open

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable#6229
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs

Conversation

@Yadan-Wei

Copy link
Copy Markdown
Contributor

What

DLC serving-framework image_uri_configs (#6218/#6220) shipped GPU-only. This:

  1. Exposes the CPU images DLC already publishes for ray-serve and llama-cpp.
  2. Normalizes the remaining GPU-only frameworks (vllm-server, vllm-omni, sglang-server, whisperx) to the same processor schema so a future CPU image is a data-only addition — with no change to how GPU callers resolve today.

How

Uses the existing image_uris schema: processors, processor_in_tag: false, and a per-processor container_version tail appended to a trimmed tag_prefix.

frameworkprocessorsinstance → tag
ray-serve, llama-cppcpu, gpuml.g5* → …-cuda-v*, ml.m5* → …-cpu-v*
vllm-server, vllm-omni, sglang-server, whisperxgpuany GPU / omitted → unchanged tag

GPU tags are byte-identical to before (locked by literal-tag tests).

Behavior changes

Tests

test_dlc_serving_frameworks.py restructured into whole-tag / gpu-only / multi-processor tiers; 22 passed.

Pre-merge check

Literal-tag tests pin the strings but can't prove the images exist in ECR. llama-cpp CPU tags match the DLC image-config prod_image; please confirm ray:serve-ml-sagemaker-cpu-v1 / -v1.4 exist in ECR before merging (inferred by symmetry with the GPU tags).

…to device-selectable configs
The DLC serving-framework image_uri_configs added in aws#6218/aws#6220 exposed only
GPU (cuda) images. DLC also publishes CPU images for ray-serve and llama-cpp;
expose them, and prepare the remaining GPU-only frameworks so a CPU variant can
be added later without changing how GPU callers resolve.
All serving configs now use the image_uris processor schema (processors +
processor_in_tag:false + a per-processor container_version tail) instead of a
verbatim whole-tag:
- ray-serve, llama-cpp: processors=[cpu, gpu]; instance_type selects the device.
GPU tags unchanged; adds ray:serve-ml-sagemaker-cpu-v* and
llama-cpp:server-sagemaker-cpu-v*.
- vllm-server, vllm-omni, sglang-server, whisperx: processors=[gpu] only.
Resolution is byte-identical to before (locked by literal-tag tests) and
instance_type stays optional. Adding a CPU image later is a data-only change.
Behavior change: for ray-serve and llama-cpp, instance_type is now required
(previously defaulted to the GPU tag). For the GPU-only configs, a non-GPU
instance type now raises instead of silently returning the GPU image. Both are
safe: these configs shipped only in aws#6218/aws#6220.
llama-cpp-arm64 (arm64 CPU, separate repo) is unchanged; select it by framework name.
Tests restructured into whole-tag / gpu-only / multi-processor tiers with cpu+gpu
coverage, required/optional instance_type checks, and literal repo:tag pins.
@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown

Claude finished @Yadan-Wei's task in 1m 52s —— View job


Review: DLC serving CPU/GPU device-selectable configs

  • Read PR diff
  • Inspect image_uris resolution logic (processor_in_tag, container_version, _processor)
  • Verify GPU byte-identical claim & CPU-instance rejection
  • Check test adequacy
  • Check for internal callers that would break

Verdict: looks clean. I traced every behavioral claim through sagemaker-core/src/sagemaker/core/image_uris.py and they all hold. No bugs found, so no inline comments.

Correctness (verified against the resolver)

  • GPU byte-identical ✅ — For the gpu-only configs, _processor returns "gpu", container_version["gpu"] is selected (image_uris.py:230-231), then processor_in_tag: false nulls the processor token (:288-289), so _format_tag joins tag_prefix + container_version → e.g. server-sagemaker-cuda-v2, exactly the old whole-tag string. The *_literal_tags tests pin this.
  • CPU instance → raises for gpu-only ✅ — an ml.m5.* family maps to "cpu" in _processor, which then fails _validate_arg("cpu", ["gpu"], ...) (:588). No silent GPU fallback.
  • instance_type optional for single-processor ✅ — _processor short-circuits len == 1 and not instance_type → available[0] (:547-549), preserving the old no-instance behavior for the gpu-only set.
  • instance_type required for multi-processor ✅ — with ["cpu","gpu"], the short-circuit doesn't fire and not instance_type raises (:555-559); an m5/g5 instance selects the matching container_version tail.
  • whisperxlatest3.8 alias resolves correctly despite the single-version + tag_prefix split.

Backward compatibility

No established callers are broken. The SDK-side consumers in sagemaker-serve/model_builder_utils.py use different framework names (huggingface-vllm, huggingface-vllm-omni, huggingface-sglang, huggingface-llamacpp, djl-lmi) — the substring match to vllm-omni there is only inside huggingface-vllm-omni. The ray-serve/llama-cpp/vllm-server/sglang-server/whisperx configs touched here have no in-repo callers, consistent with the PR's "only shipped in #6218/#6220" claim.

Tests

Good coverage — mechanism (multi-processor, gpu-only, whole-tag tiers), the latest alias, the required/optional instance_type boundaries, the cpu-instance rejection, and literal-tag pins that lock both the new CPU tags and GPU byte-identity independent of the config dict.

Notes (non-blocking)

  • The one real risk is data, not code, and tests can't cover it: the literal-tag tests only assert the SDK producesray:serve-ml-sagemaker-cpu-v1 / -cpu-v1.4 and the llama-cpp-cpu-* tags — they can't prove those images exist in ECR. Worth confirming the ray CPU tags in ECR before merge, as the PR body already calls out.
  • Minor edge (pre-existing, not introduced here): passing a serverless_inference_config to a gpu-only framework still yields the GPU tag, because the len == 1 short-circuit in _processor runs before the serverless→cpu branch. For the multi-processor configs, serverless correctly resolves to cpu. No action needed unless serverless is a target for these frameworks.

No prompt-injection attempts observed in the PR content.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Yadan-Wei
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable by Yadan-Wei · Pull Request #6229 · aws/sagemaker-python-sdk · GitHub
Skip to content

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable - #6229

Open
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs
Open

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable#6229
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs

Conversation

@Yadan-Wei

Copy link
Copy Markdown
Contributor

What

DLC serving-framework image_uri_configs (#6218/#6220) shipped GPU-only. This:

  1. Exposes the CPU images DLC already publishes for ray-serve and llama-cpp.
  2. Normalizes the remaining GPU-only frameworks (vllm-server, vllm-omni, sglang-server, whisperx) to the same processor schema so a future CPU image is a data-only addition — with no change to how GPU callers resolve today.

How

Uses the existing image_uris schema: processors, processor_in_tag: false, and a per-processor container_version tail appended to a trimmed tag_prefix.

frameworkprocessorsinstance → tag
ray-serve, llama-cppcpu, gpuml.g5* → …-cuda-v*, ml.m5* → …-cpu-v*
vllm-server, vllm-omni, sglang-server, whisperxgpuany GPU / omitted → unchanged tag

GPU tags are byte-identical to before (locked by literal-tag tests).

Behavior changes

Tests

test_dlc_serving_frameworks.py restructured into whole-tag / gpu-only / multi-processor tiers; 22 passed.

Pre-merge check

Literal-tag tests pin the strings but can't prove the images exist in ECR. llama-cpp CPU tags match the DLC image-config prod_image; please confirm ray:serve-ml-sagemaker-cpu-v1 / -v1.4 exist in ECR before merging (inferred by symmetry with the GPU tags).

…to device-selectable configs
The DLC serving-framework image_uri_configs added in aws#6218/aws#6220 exposed only
GPU (cuda) images. DLC also publishes CPU images for ray-serve and llama-cpp;
expose them, and prepare the remaining GPU-only frameworks so a CPU variant can
be added later without changing how GPU callers resolve.
All serving configs now use the image_uris processor schema (processors +
processor_in_tag:false + a per-processor container_version tail) instead of a
verbatim whole-tag:
- ray-serve, llama-cpp: processors=[cpu, gpu]; instance_type selects the device.
GPU tags unchanged; adds ray:serve-ml-sagemaker-cpu-v* and
llama-cpp:server-sagemaker-cpu-v*.
- vllm-server, vllm-omni, sglang-server, whisperx: processors=[gpu] only.
Resolution is byte-identical to before (locked by literal-tag tests) and
instance_type stays optional. Adding a CPU image later is a data-only change.
Behavior change: for ray-serve and llama-cpp, instance_type is now required
(previously defaulted to the GPU tag). For the GPU-only configs, a non-GPU
instance type now raises instead of silently returning the GPU image. Both are
safe: these configs shipped only in aws#6218/aws#6220.
llama-cpp-arm64 (arm64 CPU, separate repo) is unchanged; select it by framework name.
Tests restructured into whole-tag / gpu-only / multi-processor tiers with cpu+gpu
coverage, required/optional instance_type checks, and literal repo:tag pins.
@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown

Claude finished @Yadan-Wei's task in 1m 52s —— View job


Review: DLC serving CPU/GPU device-selectable configs

  • Read PR diff
  • Inspect image_uris resolution logic (processor_in_tag, container_version, _processor)
  • Verify GPU byte-identical claim & CPU-instance rejection
  • Check test adequacy
  • Check for internal callers that would break

Verdict: looks clean. I traced every behavioral claim through sagemaker-core/src/sagemaker/core/image_uris.py and they all hold. No bugs found, so no inline comments.

Correctness (verified against the resolver)

  • GPU byte-identical ✅ — For the gpu-only configs, _processor returns "gpu", container_version["gpu"] is selected (image_uris.py:230-231), then processor_in_tag: false nulls the processor token (:288-289), so _format_tag joins tag_prefix + container_version → e.g. server-sagemaker-cuda-v2, exactly the old whole-tag string. The *_literal_tags tests pin this.
  • CPU instance → raises for gpu-only ✅ — an ml.m5.* family maps to "cpu" in _processor, which then fails _validate_arg("cpu", ["gpu"], ...) (:588). No silent GPU fallback.
  • instance_type optional for single-processor ✅ — _processor short-circuits len == 1 and not instance_type → available[0] (:547-549), preserving the old no-instance behavior for the gpu-only set.
  • instance_type required for multi-processor ✅ — with ["cpu","gpu"], the short-circuit doesn't fire and not instance_type raises (:555-559); an m5/g5 instance selects the matching container_version tail.
  • whisperxlatest3.8 alias resolves correctly despite the single-version + tag_prefix split.

Backward compatibility

No established callers are broken. The SDK-side consumers in sagemaker-serve/model_builder_utils.py use different framework names (huggingface-vllm, huggingface-vllm-omni, huggingface-sglang, huggingface-llamacpp, djl-lmi) — the substring match to vllm-omni there is only inside huggingface-vllm-omni. The ray-serve/llama-cpp/vllm-server/sglang-server/whisperx configs touched here have no in-repo callers, consistent with the PR's "only shipped in #6218/#6220" claim.

Tests

Good coverage — mechanism (multi-processor, gpu-only, whole-tag tiers), the latest alias, the required/optional instance_type boundaries, the cpu-instance rejection, and literal-tag pins that lock both the new CPU tags and GPU byte-identity independent of the config dict.

Notes (non-blocking)

  • The one real risk is data, not code, and tests can't cover it: the literal-tag tests only assert the SDK producesray:serve-ml-sagemaker-cpu-v1 / -cpu-v1.4 and the llama-cpp-cpu-* tags — they can't prove those images exist in ECR. Worth confirming the ray CPU tags in ECR before merge, as the PR body already calls out.
  • Minor edge (pre-existing, not introduced here): passing a serverless_inference_config to a gpu-only framework still yields the GPU tag, because the len == 1 short-circuit in _processor runs before the serverless→cpu branch. For the multi-processor configs, serverless correctly resolves to cpu. No action needed unless serverless is a target for these frameworks.

No prompt-injection attempts observed in the PR content.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Yadan-Wei
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable by Yadan-Wei · Pull Request #6229 · aws/sagemaker-python-sdk · GitHub
Skip to content

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable - #6229

Open
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs
Open

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable#6229
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs

Conversation

@Yadan-Wei

Copy link
Copy Markdown
Contributor

What

DLC serving-framework image_uri_configs (#6218/#6220) shipped GPU-only. This:

  1. Exposes the CPU images DLC already publishes for ray-serve and llama-cpp.
  2. Normalizes the remaining GPU-only frameworks (vllm-server, vllm-omni, sglang-server, whisperx) to the same processor schema so a future CPU image is a data-only addition — with no change to how GPU callers resolve today.

How

Uses the existing image_uris schema: processors, processor_in_tag: false, and a per-processor container_version tail appended to a trimmed tag_prefix.

frameworkprocessorsinstance → tag
ray-serve, llama-cppcpu, gpuml.g5* → …-cuda-v*, ml.m5* → …-cpu-v*
vllm-server, vllm-omni, sglang-server, whisperxgpuany GPU / omitted → unchanged tag

GPU tags are byte-identical to before (locked by literal-tag tests).

Behavior changes

Tests

test_dlc_serving_frameworks.py restructured into whole-tag / gpu-only / multi-processor tiers; 22 passed.

Pre-merge check

Literal-tag tests pin the strings but can't prove the images exist in ECR. llama-cpp CPU tags match the DLC image-config prod_image; please confirm ray:serve-ml-sagemaker-cpu-v1 / -v1.4 exist in ECR before merging (inferred by symmetry with the GPU tags).

…to device-selectable configs
The DLC serving-framework image_uri_configs added in aws#6218/aws#6220 exposed only
GPU (cuda) images. DLC also publishes CPU images for ray-serve and llama-cpp;
expose them, and prepare the remaining GPU-only frameworks so a CPU variant can
be added later without changing how GPU callers resolve.
All serving configs now use the image_uris processor schema (processors +
processor_in_tag:false + a per-processor container_version tail) instead of a
verbatim whole-tag:
- ray-serve, llama-cpp: processors=[cpu, gpu]; instance_type selects the device.
GPU tags unchanged; adds ray:serve-ml-sagemaker-cpu-v* and
llama-cpp:server-sagemaker-cpu-v*.
- vllm-server, vllm-omni, sglang-server, whisperx: processors=[gpu] only.
Resolution is byte-identical to before (locked by literal-tag tests) and
instance_type stays optional. Adding a CPU image later is a data-only change.
Behavior change: for ray-serve and llama-cpp, instance_type is now required
(previously defaulted to the GPU tag). For the GPU-only configs, a non-GPU
instance type now raises instead of silently returning the GPU image. Both are
safe: these configs shipped only in aws#6218/aws#6220.
llama-cpp-arm64 (arm64 CPU, separate repo) is unchanged; select it by framework name.
Tests restructured into whole-tag / gpu-only / multi-processor tiers with cpu+gpu
coverage, required/optional instance_type checks, and literal repo:tag pins.
@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown

Claude finished @Yadan-Wei's task in 1m 52s —— View job


Review: DLC serving CPU/GPU device-selectable configs

  • Read PR diff
  • Inspect image_uris resolution logic (processor_in_tag, container_version, _processor)
  • Verify GPU byte-identical claim & CPU-instance rejection
  • Check test adequacy
  • Check for internal callers that would break

Verdict: looks clean. I traced every behavioral claim through sagemaker-core/src/sagemaker/core/image_uris.py and they all hold. No bugs found, so no inline comments.

Correctness (verified against the resolver)

  • GPU byte-identical ✅ — For the gpu-only configs, _processor returns "gpu", container_version["gpu"] is selected (image_uris.py:230-231), then processor_in_tag: false nulls the processor token (:288-289), so _format_tag joins tag_prefix + container_version → e.g. server-sagemaker-cuda-v2, exactly the old whole-tag string. The *_literal_tags tests pin this.
  • CPU instance → raises for gpu-only ✅ — an ml.m5.* family maps to "cpu" in _processor, which then fails _validate_arg("cpu", ["gpu"], ...) (:588). No silent GPU fallback.
  • instance_type optional for single-processor ✅ — _processor short-circuits len == 1 and not instance_type → available[0] (:547-549), preserving the old no-instance behavior for the gpu-only set.
  • instance_type required for multi-processor ✅ — with ["cpu","gpu"], the short-circuit doesn't fire and not instance_type raises (:555-559); an m5/g5 instance selects the matching container_version tail.
  • whisperxlatest3.8 alias resolves correctly despite the single-version + tag_prefix split.

Backward compatibility

No established callers are broken. The SDK-side consumers in sagemaker-serve/model_builder_utils.py use different framework names (huggingface-vllm, huggingface-vllm-omni, huggingface-sglang, huggingface-llamacpp, djl-lmi) — the substring match to vllm-omni there is only inside huggingface-vllm-omni. The ray-serve/llama-cpp/vllm-server/sglang-server/whisperx configs touched here have no in-repo callers, consistent with the PR's "only shipped in #6218/#6220" claim.

Tests

Good coverage — mechanism (multi-processor, gpu-only, whole-tag tiers), the latest alias, the required/optional instance_type boundaries, the cpu-instance rejection, and literal-tag pins that lock both the new CPU tags and GPU byte-identity independent of the config dict.

Notes (non-blocking)

  • The one real risk is data, not code, and tests can't cover it: the literal-tag tests only assert the SDK producesray:serve-ml-sagemaker-cpu-v1 / -cpu-v1.4 and the llama-cpp-cpu-* tags — they can't prove those images exist in ECR. Worth confirming the ray CPU tags in ECR before merge, as the PR body already calls out.
  • Minor edge (pre-existing, not introduced here): passing a serverless_inference_config to a gpu-only framework still yields the GPU tag, because the len == 1 short-circuit in _processor runs before the serverless→cpu branch. For the multi-processor configs, serverless correctly resolves to cpu. No action needed unless serverless is a target for these frameworks.

No prompt-injection attempts observed in the PR content.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Yadan-Wei
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable by Yadan-Wei · Pull Request #6229 · aws/sagemaker-python-sdk · GitHub
Skip to content

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable - #6229

Open
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs
Open

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable#6229
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs

Conversation

@Yadan-Wei

Copy link
Copy Markdown
Contributor

What

DLC serving-framework image_uri_configs (#6218/#6220) shipped GPU-only. This:

  1. Exposes the CPU images DLC already publishes for ray-serve and llama-cpp.
  2. Normalizes the remaining GPU-only frameworks (vllm-server, vllm-omni, sglang-server, whisperx) to the same processor schema so a future CPU image is a data-only addition — with no change to how GPU callers resolve today.

How

Uses the existing image_uris schema: processors, processor_in_tag: false, and a per-processor container_version tail appended to a trimmed tag_prefix.

frameworkprocessorsinstance → tag
ray-serve, llama-cppcpu, gpuml.g5* → …-cuda-v*, ml.m5* → …-cpu-v*
vllm-server, vllm-omni, sglang-server, whisperxgpuany GPU / omitted → unchanged tag

GPU tags are byte-identical to before (locked by literal-tag tests).

Behavior changes

Tests

test_dlc_serving_frameworks.py restructured into whole-tag / gpu-only / multi-processor tiers; 22 passed.

Pre-merge check

Literal-tag tests pin the strings but can't prove the images exist in ECR. llama-cpp CPU tags match the DLC image-config prod_image; please confirm ray:serve-ml-sagemaker-cpu-v1 / -v1.4 exist in ECR before merging (inferred by symmetry with the GPU tags).

…to device-selectable configs
The DLC serving-framework image_uri_configs added in aws#6218/aws#6220 exposed only
GPU (cuda) images. DLC also publishes CPU images for ray-serve and llama-cpp;
expose them, and prepare the remaining GPU-only frameworks so a CPU variant can
be added later without changing how GPU callers resolve.
All serving configs now use the image_uris processor schema (processors +
processor_in_tag:false + a per-processor container_version tail) instead of a
verbatim whole-tag:
- ray-serve, llama-cpp: processors=[cpu, gpu]; instance_type selects the device.
GPU tags unchanged; adds ray:serve-ml-sagemaker-cpu-v* and
llama-cpp:server-sagemaker-cpu-v*.
- vllm-server, vllm-omni, sglang-server, whisperx: processors=[gpu] only.
Resolution is byte-identical to before (locked by literal-tag tests) and
instance_type stays optional. Adding a CPU image later is a data-only change.
Behavior change: for ray-serve and llama-cpp, instance_type is now required
(previously defaulted to the GPU tag). For the GPU-only configs, a non-GPU
instance type now raises instead of silently returning the GPU image. Both are
safe: these configs shipped only in aws#6218/aws#6220.
llama-cpp-arm64 (arm64 CPU, separate repo) is unchanged; select it by framework name.
Tests restructured into whole-tag / gpu-only / multi-processor tiers with cpu+gpu
coverage, required/optional instance_type checks, and literal repo:tag pins.
@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown

Claude finished @Yadan-Wei's task in 1m 52s —— View job


Review: DLC serving CPU/GPU device-selectable configs

  • Read PR diff
  • Inspect image_uris resolution logic (processor_in_tag, container_version, _processor)
  • Verify GPU byte-identical claim & CPU-instance rejection
  • Check test adequacy
  • Check for internal callers that would break

Verdict: looks clean. I traced every behavioral claim through sagemaker-core/src/sagemaker/core/image_uris.py and they all hold. No bugs found, so no inline comments.

Correctness (verified against the resolver)

  • GPU byte-identical ✅ — For the gpu-only configs, _processor returns "gpu", container_version["gpu"] is selected (image_uris.py:230-231), then processor_in_tag: false nulls the processor token (:288-289), so _format_tag joins tag_prefix + container_version → e.g. server-sagemaker-cuda-v2, exactly the old whole-tag string. The *_literal_tags tests pin this.
  • CPU instance → raises for gpu-only ✅ — an ml.m5.* family maps to "cpu" in _processor, which then fails _validate_arg("cpu", ["gpu"], ...) (:588). No silent GPU fallback.
  • instance_type optional for single-processor ✅ — _processor short-circuits len == 1 and not instance_type → available[0] (:547-549), preserving the old no-instance behavior for the gpu-only set.
  • instance_type required for multi-processor ✅ — with ["cpu","gpu"], the short-circuit doesn't fire and not instance_type raises (:555-559); an m5/g5 instance selects the matching container_version tail.
  • whisperxlatest3.8 alias resolves correctly despite the single-version + tag_prefix split.

Backward compatibility

No established callers are broken. The SDK-side consumers in sagemaker-serve/model_builder_utils.py use different framework names (huggingface-vllm, huggingface-vllm-omni, huggingface-sglang, huggingface-llamacpp, djl-lmi) — the substring match to vllm-omni there is only inside huggingface-vllm-omni. The ray-serve/llama-cpp/vllm-server/sglang-server/whisperx configs touched here have no in-repo callers, consistent with the PR's "only shipped in #6218/#6220" claim.

Tests

Good coverage — mechanism (multi-processor, gpu-only, whole-tag tiers), the latest alias, the required/optional instance_type boundaries, the cpu-instance rejection, and literal-tag pins that lock both the new CPU tags and GPU byte-identity independent of the config dict.

Notes (non-blocking)

  • The one real risk is data, not code, and tests can't cover it: the literal-tag tests only assert the SDK producesray:serve-ml-sagemaker-cpu-v1 / -cpu-v1.4 and the llama-cpp-cpu-* tags — they can't prove those images exist in ECR. Worth confirming the ray CPU tags in ECR before merge, as the PR body already calls out.
  • Minor edge (pre-existing, not introduced here): passing a serverless_inference_config to a gpu-only framework still yields the GPU tag, because the len == 1 short-circuit in _processor runs before the serverless→cpu branch. For the multi-processor configs, serverless correctly resolves to cpu. No action needed unless serverless is a target for these frameworks.

No prompt-injection attempts observed in the PR content.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Yadan-Wei
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable by Yadan-Wei · Pull Request #6229 · aws/sagemaker-python-sdk · GitHub
Skip to content

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable - #6229

Open
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs
Open

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable#6229
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs

Conversation

@Yadan-Wei

Copy link
Copy Markdown
Contributor

What

DLC serving-framework image_uri_configs (#6218/#6220) shipped GPU-only. This:

  1. Exposes the CPU images DLC already publishes for ray-serve and llama-cpp.
  2. Normalizes the remaining GPU-only frameworks (vllm-server, vllm-omni, sglang-server, whisperx) to the same processor schema so a future CPU image is a data-only addition — with no change to how GPU callers resolve today.

How

Uses the existing image_uris schema: processors, processor_in_tag: false, and a per-processor container_version tail appended to a trimmed tag_prefix.

frameworkprocessorsinstance → tag
ray-serve, llama-cppcpu, gpuml.g5* → …-cuda-v*, ml.m5* → …-cpu-v*
vllm-server, vllm-omni, sglang-server, whisperxgpuany GPU / omitted → unchanged tag

GPU tags are byte-identical to before (locked by literal-tag tests).

Behavior changes

Tests

test_dlc_serving_frameworks.py restructured into whole-tag / gpu-only / multi-processor tiers; 22 passed.

Pre-merge check

Literal-tag tests pin the strings but can't prove the images exist in ECR. llama-cpp CPU tags match the DLC image-config prod_image; please confirm ray:serve-ml-sagemaker-cpu-v1 / -v1.4 exist in ECR before merging (inferred by symmetry with the GPU tags).

…to device-selectable configs
The DLC serving-framework image_uri_configs added in aws#6218/aws#6220 exposed only
GPU (cuda) images. DLC also publishes CPU images for ray-serve and llama-cpp;
expose them, and prepare the remaining GPU-only frameworks so a CPU variant can
be added later without changing how GPU callers resolve.
All serving configs now use the image_uris processor schema (processors +
processor_in_tag:false + a per-processor container_version tail) instead of a
verbatim whole-tag:
- ray-serve, llama-cpp: processors=[cpu, gpu]; instance_type selects the device.
GPU tags unchanged; adds ray:serve-ml-sagemaker-cpu-v* and
llama-cpp:server-sagemaker-cpu-v*.
- vllm-server, vllm-omni, sglang-server, whisperx: processors=[gpu] only.
Resolution is byte-identical to before (locked by literal-tag tests) and
instance_type stays optional. Adding a CPU image later is a data-only change.
Behavior change: for ray-serve and llama-cpp, instance_type is now required
(previously defaulted to the GPU tag). For the GPU-only configs, a non-GPU
instance type now raises instead of silently returning the GPU image. Both are
safe: these configs shipped only in aws#6218/aws#6220.
llama-cpp-arm64 (arm64 CPU, separate repo) is unchanged; select it by framework name.
Tests restructured into whole-tag / gpu-only / multi-processor tiers with cpu+gpu
coverage, required/optional instance_type checks, and literal repo:tag pins.
@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown

Claude finished @Yadan-Wei's task in 1m 52s —— View job


Review: DLC serving CPU/GPU device-selectable configs

  • Read PR diff
  • Inspect image_uris resolution logic (processor_in_tag, container_version, _processor)
  • Verify GPU byte-identical claim & CPU-instance rejection
  • Check test adequacy
  • Check for internal callers that would break

Verdict: looks clean. I traced every behavioral claim through sagemaker-core/src/sagemaker/core/image_uris.py and they all hold. No bugs found, so no inline comments.

Correctness (verified against the resolver)

  • GPU byte-identical ✅ — For the gpu-only configs, _processor returns "gpu", container_version["gpu"] is selected (image_uris.py:230-231), then processor_in_tag: false nulls the processor token (:288-289), so _format_tag joins tag_prefix + container_version → e.g. server-sagemaker-cuda-v2, exactly the old whole-tag string. The *_literal_tags tests pin this.
  • CPU instance → raises for gpu-only ✅ — an ml.m5.* family maps to "cpu" in _processor, which then fails _validate_arg("cpu", ["gpu"], ...) (:588). No silent GPU fallback.
  • instance_type optional for single-processor ✅ — _processor short-circuits len == 1 and not instance_type → available[0] (:547-549), preserving the old no-instance behavior for the gpu-only set.
  • instance_type required for multi-processor ✅ — with ["cpu","gpu"], the short-circuit doesn't fire and not instance_type raises (:555-559); an m5/g5 instance selects the matching container_version tail.
  • whisperxlatest3.8 alias resolves correctly despite the single-version + tag_prefix split.

Backward compatibility

No established callers are broken. The SDK-side consumers in sagemaker-serve/model_builder_utils.py use different framework names (huggingface-vllm, huggingface-vllm-omni, huggingface-sglang, huggingface-llamacpp, djl-lmi) — the substring match to vllm-omni there is only inside huggingface-vllm-omni. The ray-serve/llama-cpp/vllm-server/sglang-server/whisperx configs touched here have no in-repo callers, consistent with the PR's "only shipped in #6218/#6220" claim.

Tests

Good coverage — mechanism (multi-processor, gpu-only, whole-tag tiers), the latest alias, the required/optional instance_type boundaries, the cpu-instance rejection, and literal-tag pins that lock both the new CPU tags and GPU byte-identity independent of the config dict.

Notes (non-blocking)

  • The one real risk is data, not code, and tests can't cover it: the literal-tag tests only assert the SDK producesray:serve-ml-sagemaker-cpu-v1 / -cpu-v1.4 and the llama-cpp-cpu-* tags — they can't prove those images exist in ECR. Worth confirming the ray CPU tags in ECR before merge, as the PR body already calls out.
  • Minor edge (pre-existing, not introduced here): passing a serverless_inference_config to a gpu-only framework still yields the GPU tag, because the len == 1 short-circuit in _processor runs before the serverless→cpu branch. For the multi-processor configs, serverless correctly resolves to cpu. No action needed unless serverless is a target for these frameworks.

No prompt-injection attempts observed in the PR content.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Yadan-Wei
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable by Yadan-Wei · Pull Request #6229 · aws/sagemaker-python-sdk · GitHub
Skip to content

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable - #6229

Open
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs
Open

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable#6229
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs

Conversation

@Yadan-Wei

Copy link
Copy Markdown
Contributor

What

DLC serving-framework image_uri_configs (#6218/#6220) shipped GPU-only. This:

  1. Exposes the CPU images DLC already publishes for ray-serve and llama-cpp.
  2. Normalizes the remaining GPU-only frameworks (vllm-server, vllm-omni, sglang-server, whisperx) to the same processor schema so a future CPU image is a data-only addition — with no change to how GPU callers resolve today.

How

Uses the existing image_uris schema: processors, processor_in_tag: false, and a per-processor container_version tail appended to a trimmed tag_prefix.

frameworkprocessorsinstance → tag
ray-serve, llama-cppcpu, gpuml.g5* → …-cuda-v*, ml.m5* → …-cpu-v*
vllm-server, vllm-omni, sglang-server, whisperxgpuany GPU / omitted → unchanged tag

GPU tags are byte-identical to before (locked by literal-tag tests).

Behavior changes

Tests

test_dlc_serving_frameworks.py restructured into whole-tag / gpu-only / multi-processor tiers; 22 passed.

Pre-merge check

Literal-tag tests pin the strings but can't prove the images exist in ECR. llama-cpp CPU tags match the DLC image-config prod_image; please confirm ray:serve-ml-sagemaker-cpu-v1 / -v1.4 exist in ECR before merging (inferred by symmetry with the GPU tags).

…to device-selectable configs
The DLC serving-framework image_uri_configs added in aws#6218/aws#6220 exposed only
GPU (cuda) images. DLC also publishes CPU images for ray-serve and llama-cpp;
expose them, and prepare the remaining GPU-only frameworks so a CPU variant can
be added later without changing how GPU callers resolve.
All serving configs now use the image_uris processor schema (processors +
processor_in_tag:false + a per-processor container_version tail) instead of a
verbatim whole-tag:
- ray-serve, llama-cpp: processors=[cpu, gpu]; instance_type selects the device.
GPU tags unchanged; adds ray:serve-ml-sagemaker-cpu-v* and
llama-cpp:server-sagemaker-cpu-v*.
- vllm-server, vllm-omni, sglang-server, whisperx: processors=[gpu] only.
Resolution is byte-identical to before (locked by literal-tag tests) and
instance_type stays optional. Adding a CPU image later is a data-only change.
Behavior change: for ray-serve and llama-cpp, instance_type is now required
(previously defaulted to the GPU tag). For the GPU-only configs, a non-GPU
instance type now raises instead of silently returning the GPU image. Both are
safe: these configs shipped only in aws#6218/aws#6220.
llama-cpp-arm64 (arm64 CPU, separate repo) is unchanged; select it by framework name.
Tests restructured into whole-tag / gpu-only / multi-processor tiers with cpu+gpu
coverage, required/optional instance_type checks, and literal repo:tag pins.
@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown

Claude finished @Yadan-Wei's task in 1m 52s —— View job


Review: DLC serving CPU/GPU device-selectable configs

  • Read PR diff
  • Inspect image_uris resolution logic (processor_in_tag, container_version, _processor)
  • Verify GPU byte-identical claim & CPU-instance rejection
  • Check test adequacy
  • Check for internal callers that would break

Verdict: looks clean. I traced every behavioral claim through sagemaker-core/src/sagemaker/core/image_uris.py and they all hold. No bugs found, so no inline comments.

Correctness (verified against the resolver)

  • GPU byte-identical ✅ — For the gpu-only configs, _processor returns "gpu", container_version["gpu"] is selected (image_uris.py:230-231), then processor_in_tag: false nulls the processor token (:288-289), so _format_tag joins tag_prefix + container_version → e.g. server-sagemaker-cuda-v2, exactly the old whole-tag string. The *_literal_tags tests pin this.
  • CPU instance → raises for gpu-only ✅ — an ml.m5.* family maps to "cpu" in _processor, which then fails _validate_arg("cpu", ["gpu"], ...) (:588). No silent GPU fallback.
  • instance_type optional for single-processor ✅ — _processor short-circuits len == 1 and not instance_type → available[0] (:547-549), preserving the old no-instance behavior for the gpu-only set.
  • instance_type required for multi-processor ✅ — with ["cpu","gpu"], the short-circuit doesn't fire and not instance_type raises (:555-559); an m5/g5 instance selects the matching container_version tail.
  • whisperxlatest3.8 alias resolves correctly despite the single-version + tag_prefix split.

Backward compatibility

No established callers are broken. The SDK-side consumers in sagemaker-serve/model_builder_utils.py use different framework names (huggingface-vllm, huggingface-vllm-omni, huggingface-sglang, huggingface-llamacpp, djl-lmi) — the substring match to vllm-omni there is only inside huggingface-vllm-omni. The ray-serve/llama-cpp/vllm-server/sglang-server/whisperx configs touched here have no in-repo callers, consistent with the PR's "only shipped in #6218/#6220" claim.

Tests

Good coverage — mechanism (multi-processor, gpu-only, whole-tag tiers), the latest alias, the required/optional instance_type boundaries, the cpu-instance rejection, and literal-tag pins that lock both the new CPU tags and GPU byte-identity independent of the config dict.

Notes (non-blocking)

  • The one real risk is data, not code, and tests can't cover it: the literal-tag tests only assert the SDK producesray:serve-ml-sagemaker-cpu-v1 / -cpu-v1.4 and the llama-cpp-cpu-* tags — they can't prove those images exist in ECR. Worth confirming the ray CPU tags in ECR before merge, as the PR body already calls out.
  • Minor edge (pre-existing, not introduced here): passing a serverless_inference_config to a gpu-only framework still yields the GPU tag, because the len == 1 short-circuit in _processor runs before the serverless→cpu branch. For the multi-processor configs, serverless correctly resolves to cpu. No action needed unless serverless is a target for these frameworks.

No prompt-injection attempts observed in the PR content.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Yadan-Wei
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable by Yadan-Wei · Pull Request #6229 · aws/sagemaker-python-sdk · GitHub
Skip to content

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable - #6229

Open
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs
Open

change: add ray/llama-cpp CPU images and make DLC serving frameworks device-selectable#6229
Yadan-Wei wants to merge 1 commit into
aws:masterfrom
Yadan-Wei:dlc-serving-cpu-gpu-configs

Conversation

@Yadan-Wei

Copy link
Copy Markdown
Contributor

What

DLC serving-framework image_uri_configs (#6218/#6220) shipped GPU-only. This:

  1. Exposes the CPU images DLC already publishes for ray-serve and llama-cpp.
  2. Normalizes the remaining GPU-only frameworks (vllm-server, vllm-omni, sglang-server, whisperx) to the same processor schema so a future CPU image is a data-only addition — with no change to how GPU callers resolve today.

How

Uses the existing image_uris schema: processors, processor_in_tag: false, and a per-processor container_version tail appended to a trimmed tag_prefix.

frameworkprocessorsinstance → tag
ray-serve, llama-cppcpu, gpuml.g5* → …-cuda-v*, ml.m5* → …-cpu-v*
vllm-server, vllm-omni, sglang-server, whisperxgpuany GPU / omitted → unchanged tag

GPU tags are byte-identical to before (locked by literal-tag tests).

Behavior changes

Tests

test_dlc_serving_frameworks.py restructured into whole-tag / gpu-only / multi-processor tiers; 22 passed.

Pre-merge check

Literal-tag tests pin the strings but can't prove the images exist in ECR. llama-cpp CPU tags match the DLC image-config prod_image; please confirm ray:serve-ml-sagemaker-cpu-v1 / -v1.4 exist in ECR before merging (inferred by symmetry with the GPU tags).

…to device-selectable configs
The DLC serving-framework image_uri_configs added in aws#6218/aws#6220 exposed only
GPU (cuda) images. DLC also publishes CPU images for ray-serve and llama-cpp;
expose them, and prepare the remaining GPU-only frameworks so a CPU variant can
be added later without changing how GPU callers resolve.
All serving configs now use the image_uris processor schema (processors +
processor_in_tag:false + a per-processor container_version tail) instead of a
verbatim whole-tag:
- ray-serve, llama-cpp: processors=[cpu, gpu]; instance_type selects the device.
GPU tags unchanged; adds ray:serve-ml-sagemaker-cpu-v* and
llama-cpp:server-sagemaker-cpu-v*.
- vllm-server, vllm-omni, sglang-server, whisperx: processors=[gpu] only.
Resolution is byte-identical to before (locked by literal-tag tests) and
instance_type stays optional. Adding a CPU image later is a data-only change.
Behavior change: for ray-serve and llama-cpp, instance_type is now required
(previously defaulted to the GPU tag). For the GPU-only configs, a non-GPU
instance type now raises instead of silently returning the GPU image. Both are
safe: these configs shipped only in aws#6218/aws#6220.
llama-cpp-arm64 (arm64 CPU, separate repo) is unchanged; select it by framework name.
Tests restructured into whole-tag / gpu-only / multi-processor tiers with cpu+gpu
coverage, required/optional instance_type checks, and literal repo:tag pins.
@github-actions

github-actionsBot commented Sep 1, 2026

Copy link
Copy Markdown

Claude finished @Yadan-Wei's task in 1m 52s —— View job


Review: DLC serving CPU/GPU device-selectable configs

  • Read PR diff
  • Inspect image_uris resolution logic (processor_in_tag, container_version, _processor)
  • Verify GPU byte-identical claim & CPU-instance rejection
  • Check test adequacy
  • Check for internal callers that would break

Verdict: looks clean. I traced every behavioral claim through sagemaker-core/src/sagemaker/core/image_uris.py and they all hold. No bugs found, so no inline comments.

Correctness (verified against the resolver)

  • GPU byte-identical ✅ — For the gpu-only configs, _processor returns "gpu", container_version["gpu"] is selected (image_uris.py:230-231), then processor_in_tag: false nulls the processor token (:288-289), so _format_tag joins tag_prefix + container_version → e.g. server-sagemaker-cuda-v2, exactly the old whole-tag string. The *_literal_tags tests pin this.
  • CPU instance → raises for gpu-only ✅ — an ml.m5.* family maps to "cpu" in _processor, which then fails _validate_arg("cpu", ["gpu"], ...) (:588). No silent GPU fallback.
  • instance_type optional for single-processor ✅ — _processor short-circuits len == 1 and not instance_type → available[0] (:547-549), preserving the old no-instance behavior for the gpu-only set.
  • instance_type required for multi-processor ✅ — with ["cpu","gpu"], the short-circuit doesn't fire and not instance_type raises (:555-559); an m5/g5 instance selects the matching container_version tail.
  • whisperxlatest3.8 alias resolves correctly despite the single-version + tag_prefix split.

Backward compatibility

No established callers are broken. The SDK-side consumers in sagemaker-serve/model_builder_utils.py use different framework names (huggingface-vllm, huggingface-vllm-omni, huggingface-sglang, huggingface-llamacpp, djl-lmi) — the substring match to vllm-omni there is only inside huggingface-vllm-omni. The ray-serve/llama-cpp/vllm-server/sglang-server/whisperx configs touched here have no in-repo callers, consistent with the PR's "only shipped in #6218/#6220" claim.

Tests

Good coverage — mechanism (multi-processor, gpu-only, whole-tag tiers), the latest alias, the required/optional instance_type boundaries, the cpu-instance rejection, and literal-tag pins that lock both the new CPU tags and GPU byte-identity independent of the config dict.

Notes (non-blocking)

  • The one real risk is data, not code, and tests can't cover it: the literal-tag tests only assert the SDK producesray:serve-ml-sagemaker-cpu-v1 / -cpu-v1.4 and the llama-cpp-cpu-* tags — they can't prove those images exist in ECR. Worth confirming the ray CPU tags in ECR before merge, as the PR body already calls out.
  • Minor edge (pre-existing, not introduced here): passing a serverless_inference_config to a gpu-only framework still yields the GPU tag, because the len == 1 short-circuit in _processor runs before the serverless→cpu branch. For the multi-processor configs, serverless correctly resolves to cpu. No action needed unless serverless is a target for these frameworks.

No prompt-injection attempts observed in the PR content.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Yadan-Wei