fix(vllm): account for pipeline-parallel size in per_gpu_throughput#258
Open
atnair-amd wants to merge 5 commits into
Open
fix(vllm): account for pipeline-parallel size in per_gpu_throughput#258atnair-amd wants to merge 5 commits into
atnair-amd wants to merge 5 commits into
Conversation
atnair-amd
force-pushed
the
atnair/vllm-p1-metrics
branch
from
July 16, 2026 21:13
0431712 to
cda0107
Compare
atnair-amd
marked this pull request as ready for review
July 16, 2026 21:47
…required ATOM has no pipeline-parallel concept (single-node TP only), so pp=1 preserves the prior per_gpu_throughput numeric behavior. Also widen the Ray-backend regression test to assert pp explicitly across pp=1/2 so a broken pp passthrough can't slip through silently again.
… became required" This reverts commit a2eb3c4.
atnair-amd
force-pushed
the
atnair/vllm-p1-metrics
branch
from
July 17, 2026 19:01
cda0107 to
005b5e4
Compare
Adds test_gpu_metric (one HTML row per GPU metric per sweep cell), GPU pre/post-load VRAM snapshots and background amd-smi polling during test_vllm_inference, and extends threshold-coverage validation to gate gpu.* metrics alongside client.* metrics. amd-smi runs fine from inside the benchmark container (GPU device files are passed through), so gpu.py calls orch.exec/exec_on_head directly like every other command in the suite -- no host-vs-container bypass routing needed.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
per_gpu_throughputinto_client_metricsdivided total token throughput bytpalone, undercounting GPU count on any pipeline-parallel run (actual GPUs =tp * pp).ppis now a required keyword-only argument.ppexplicitly (subTest overpp=1,2) so a brokenpppassthrough can't silently regress again — the prior version only assertedtp/isl.to_client_metricscall sites are not updated for the new requiredppkwarg and are left broken by this change (out of scope here).test_vllm_inferencenow takes pre/post-model-load VRAM snapshots and pollsamd-smiin the background while the client runs; a newtest_gpu_metrictest reports one HTML row per GPU metric per sweep cell (peak_gpu_memory_mb,model_load_memory_mb,model_load_s,gpu_bandwidth_util_pct,gpu_compute_util_pct). Threshold-coverage validation (vllm_config_loader.py) now gatesgpu.*metrics the same way it already gatedclient.*metrics. amd-smi is invoked via the container-routedorch.exec/orch.exec_on_head— GPU device files (/dev/kfd,/dev/dri) are passed through into the container, so no host-vs-container bypass routing is needed.Test plan
python3 -m unittest discover -s cvs/lib/inference/unittests -p "test_*.py"— 188 tests, only known-out-of-scope failures:test_inferencing_config_loader.py(2 pre-existing, unrelated failures confirmed viagit stashagainst commit5134be1) andtest_inferencex_atom_orch_parse.py(3 failures caused by this PR'spprequirement — ATOM is not being updated here, tracked separately)python3 -m unittest discover -s cvs/lib/utils/unittests -p "test_*.py"— 126/126 pass (gpu.py polling/aggregation logic)python3 -m unittest discover -s cvs/core/orchestrators/unittests -p "test_*.py"— 57/57 passppfix invllm_job.pycauses the regression test to fail, confirming it actually guards the fixtp8pp2config (8-way tensor-parallel, 2-way pipeline-parallel, 2-node Ray cluster) — 43 passed, 1 skipped, rc=0. Confirmsper_gpu_throughputand the rest of the metrics pipeline compute and report correctly withpp=2on live hardware (total throughput 823.47 tok/s, TTFT/TPOT/ITL all populated in the results table).pytest --collect-onlyagainstcvs/tests/inference/vllm/vllm.pywith a real vLLM config confirmstest_gpu_metriccollects correctly (5 GPU metric rows per cell) alongside the existing 44 tests, with no collection errors.Known out-of-scope breakage
InferenceX ATOM's
to_client_metricscall sites do not passppand will fail after this change (ATOM has no pipeline-parallel concept). This is expected and intentionally left unfixed here — scope is vLLM only.