fix: fold DP_SIZE into single-node num_gpus for per-GPU throughput - #2715
Open
aistackdev wants to merge 1 commit into
Open
fix: fold DP_SIZE into single-node num_gpus for per-GPU throughput#2715aistackdev wants to merge 1 commit into
aistackdev wants to merge 1 commit into
Conversation
process_result.py's single-node branch computed num_gpus as tp_size * pp * pcp_size, with no data-parallel term. Any single-node config running --data-parallel-size N without exporting that as TP/PP/ PCP_SIZE (e.g. tensor-parallel-size 1 + data-parallel-size 2, N full replicas on N GPUs) divided total_token_throughput by 1 instead of N, inflating tput_per_gpu / output_tput_per_gpu / input_tput_per_gpu by a factor of N. Read an optional DP_SIZE env var (defaults to 1, so existing configs are unaffected) and fold it into num_gpus. Also record dp_size in the output JSON alongside the existing pp/dcp_size/pcp_size fields. Fixes SemiAnalysisAI#2714
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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
1 participant
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
Fixes #2714.
process_result.py's single-node branch computes:with no data-parallel term. Any single-node config running
--tensor-parallel-size 1 --data-parallel-size N(N full-modelreplicas on N GPUs) without also exporting that as
TP/PP/PCP_SIZEdivides
total_token_throughputby 1 instead of N, inflatingtput_per_gpu/output_tput_per_gpu/input_tput_per_gpuby afactor of N.
Fix
Read an optional
DP_SIZEenv var (defaults to"1", so everyexisting single-node config is unaffected) and fold it into
num_gpus = tp_size * pp * pcp_size * dp_size. Also recorddp_sizein the output JSON alongside the existing
pp/dcp_size/pcp_sizefields, for parity with how the other parallelism dimensions are
surfaced.
Test plan
test_throughput_per_gpu_single_node_default_dp_size—confirms
DP_SIZEunset behaves exactly as before (dp_size == 1).test_throughput_per_gpu_single_node_with_dp_size— confirmsDP_SIZE=2halvestput_per_gpu/output_tput_per_gpu/input_tput_per_gpu.test_invalid_dp_size_raises_error—DP_SIZE=0fails loud,matching the existing
PP_SIZE/DCP_SIZE/PCP_SIZEvalidation.python -m pytest utils/test_process_result.py— 53 passed (50existing + 3 new), no regressions.
Recipes that run internal
--data-parallel-size Ntoday keep N as ascript-local constant not exported to
process_result.py; wiringDP_SIZE=Nthrough for those recipes is a separate follow-up oncethis lands.
Note
Low Risk
Narrow result-aggregation math change with a default of 1, so existing runs are unchanged unless DP_SIZE is set. Does not touch auth, serving, or recipe wiring.
Overview
Fixes inflated single-node
tput_per_gpu(and related per-GPU metrics) when data-parallel replicas are used without encoding that width inTP/PP/PCP_SIZE.The single-node path now reads optional
DP_SIZE(default1), multiplies it intonum_gpus, validates it as a positive integer, and emitsdp_sizein the aggregated JSON. Existing configs that omit the env var keep the previous denominator. Recipes still need to exportDP_SIZEfor this to take effect.Reviewed by Cursor Bugbot for commit c07cfda. Bugbot is set up for automated code reviews on this repo. Configure here.