Skip to content

feat: fool-proof preflight sizing for vllm CPU deploys and llama_serv…#115

Merged
alez007 merged 5 commits into
mainfrom
feat/foolproof-preflight-vllm-llama-server
Jul 5, 2026
Merged

feat: fool-proof preflight sizing for vllm CPU deploys and llama_serv…#115
alez007 merged 5 commits into
mainfrom
feat/foolproof-preflight-vllm-llama-server

Conversation

@alez007

@alez007 alez007 commented Jul 5, 2026

Copy link
Copy Markdown
Owner

…er GPU offload

Preflight now sizes vLLM CPU deploys (max_model_len + gpu_memory_utilization against actual host RAM) and llama_server GPU offload (n_ctx + n_gpu_layers against VRAM, with an explicit ngl instead of trusting llama-server's -1 auto-fit) so a model only needs name/model/usecase/loader/num_gpus to run well on whatever hardware it lands on. Also fixes sharded-GGUF weight undercounting and adds llama_server thread alignment via num_cpus.

…er GPU offload

Preflight now sizes vLLM CPU deploys (max_model_len + gpu_memory_utilization
against actual host RAM) and llama_server GPU offload (n_ctx + n_gpu_layers
against VRAM, with an explicit ngl instead of trusting llama-server's -1
auto-fit) so a model only needs name/model/usecase/loader/num_gpus to run
well on whatever hardware it lands on. Also fixes sharded-GGUF weight
undercounting and adds llama_server thread alignment via num_cpus.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request significantly improves the preflight recommendation system for both the vllm and llama_server loaders, introducing hardware-aware auto-sizing of context lengths, GPU offload layers, and compute threads for CPU and GPU deployments. It also adds support for estimating the weight footprint of sharded GGUF models. The review feedback suggests adding defensive checks to handle undiscoverable system RAM in VllmPreflight._recommend_cpu and to prevent a potential KeyError when popping gpu_memory_utilization in split_vllm_user_overrides.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread modelship/preflight/vllm.py
Comment thread modelship/infer/infer_config.py Outdated
alez007 added 2 commits July 5, 2026 16:48
num_cpus is a Ray scheduling hint, not an enforced cap like num_gpus, so a
deploy can legitimately set it low while still running many --parallel
slots. Recommending threads=num_cpus unconditionally broke
test_concurrent_requests_are_not_serialized (num_cpus=2, parallel=4):
capping compute to 2 threads starved the 4 concurrent slots and serialized
them. Now declines the recommendation when threads would undercut parallel.
…of auto-flagging it

VllmEngineConfig.gpu_memory_utilization defaulted to 0.9 and normalize_num_gpus_and_tp
overwrote it to 0.4 for num_gpus=0 CPU deploys, marking it "set" so it still reached the
engine. That made the auto default indistinguishable from a real user override via
model_dump(exclude_unset=True), requiring a _gmu_auto private attr + split_vllm_user_overrides
just to peel it back out before merging in a preflight recommendation.

Instead, the field now defaults to None (never auto-written for the num_gpus=0 case), and
default_gpu_memory_utilization() resolves the loader-appropriate fallback (0.9 GPU / 0.4 CPU)
lazily via setdefault, after preflight and user overrides have both had a chance to win. Same
precedence, no bookkeeping flag needed to get there. The fractional-num_gpus case is unchanged
since it's a real derived value, not a default.
@alez007

alez007 commented Jul 5, 2026

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the preflight recommendation logic for both vllm and llama_server loaders to dynamically size memory and thread allocations on CPU-only deployments, preventing worker OOMs and resource starvation. Key improvements include lazy resolution of gpu_memory_utilization on CPU, automatic sizing of context lengths and GPU offload layers for GGUF models, and CPU thread alignment. Feedback on these changes highlights two critical issues: an incorrect assumption in llama_cpp.py that conflates transformer blocks with total layers for CPU-resident weight calculations, and a potential ZeroDivisionError in vllm.py if host RAM is undiscoverable.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread modelship/preflight/llama_cpp.py
Comment thread modelship/preflight/vllm.py
alez007 added 2 commits July 5, 2026 18:37
… sizing

_recommend_gpu_partial used a single cpu_layers value for both the KV-cache
budget and the weight-memory budget of CPU-resident layers. Those aren't the
same set: KV cache only exists for real transformer blocks (block_count),
but total_layers also counts one extra "layer" for the output/embedding
weights (_NON_BLOCK_LAYER_EQUIV), which has no KV cache but still occupies
host RAM whenever it isn't offloaded.

Two bugs followed: weight memory for CPU-resident layers was undercounted
by one layer_bytes whenever ngl < total_layers, and when ngl landed exactly
on block_count (every transformer block offloaded, only the output layer
left on CPU), the old cpu_layers came out to 0 and skipped the RAM budget
check entirely — happily recommending a config whose output-layer weights
don't actually fit in host RAM.

Split into cpu_blocks (block_count - ngl, for KV) and cpu_layers
(total_layers - ngl, for weights), and handle kv_ram_per_ctx == 0 as "no
context-dependent cost" rather than forcing ctx_ram to 0.
_raw_host_ram_bytes reads psutil's raw virtual_memory().total independently
of hw.ram_bytes, to match the cgroup-blind denominator vLLM's own CPU worker
uses for gpu_memory_utilization. If that probe comes back 0 (or falls back
to an unset hw.ram_bytes), _recommend_cpu_auto_gmu's
clamped_kv_bytes / denom_ram divide would raise ZeroDivisionError.
run_preflight() swallows that, so it wasn't crash-visible, but it produced a
noisy stack-trace log instead of a clean skip.

Guard denom_ram <= 0 right where it's read, before dispatching to either the
pinned- or auto-gmu path, matching the same hw.ram_bytes <= 0 early-exit
llama_cpp.py and stable_diffusion_cpp.py already use.
@alez007

alez007 commented Jul 5, 2026

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request significantly enhances the preflight hardware-sizing and recommendation logic for both the vllm and llama_server loaders, adding support for CPU-only deployments, partial GPU offloading, thread alignment, and sharded GGUF weight calculations. It also refactors gpu_memory_utilization to be resolved lazily via a fallback helper rather than being eagerly written during config initialization. A valid review comment identifies that using setdefault on config_engine_kwargs will fail to apply the fallback if the key is already present with a value of None, and suggests an explicit check instead.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread modelship/infer/vllm/vllm_infer.py
@alez007 alez007 merged commit 9d12a89 into main Jul 5, 2026
4 checks passed
Sign up for free to 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