Skip to content

[Fix] Override data parallel size for accelerator configs#611

Open
YouNeedCryDear wants to merge 1 commit into
mainfrom
fix/data-parallel-override
Open

[Fix] Override data parallel size for accelerator configs#611
YouNeedCryDear wants to merge 1 commit into
mainfrom
fix/data-parallel-override

Conversation

@YouNeedCryDear
Copy link
Copy Markdown
Collaborator

What this PR does

Adds accelerator-specific runtime argument overrides for data parallel size, covering --dp-size, --dp, --data-parallel-size, and -dp. It also extends tensor and pipeline parallel overrides to handle -tp and -pp aliases.

Why we need it

tensorParallelismOverride.dataParallelSize is part of the ServingRuntime accelerator config API, but the runtime argument override logic only applied tensor and pipeline parallel sizes. As a result, accelerator configs could set dataParallelSize without changing the generated runtime command. This makes DP override behavior match TP and PP override behavior.

Fixes # (not filed)

How to test

env GOCACHE=/private/tmp/ome-go-build-cache go test ./pkg/controller/v1beta1/inferenceservice/components -count=1

Checklist

  • Tests added/updated (if applicable)
  • Docs updated (if applicable) - N/A
  • make test passes locally

@github-actions github-actions Bot added inferenceservice InferenceService controller changes controller Controller changes tests Test changes labels May 13, 2026
@YouNeedCryDear YouNeedCryDear marked this pull request as ready for review May 13, 2026 21:51
@YouNeedCryDear YouNeedCryDear force-pushed the fix/data-parallel-override branch from 07b44cd to d2a8715 Compare May 27, 2026 23:01
@YouNeedCryDear YouNeedCryDear force-pushed the fix/data-parallel-override branch from d2a8715 to 74a7fcb Compare June 1, 2026 18:02
@YouNeedCryDear
Copy link
Copy Markdown
Collaborator Author

@slin1237 Let me know when this can be merged.

Copy link
Copy Markdown
Collaborator

@heymrbox heymrbox left a comment

Choose a reason for hiding this comment

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

Reviewed the TP/PP/DP override patch. I did not find a blocking issue.

Verification run on the PR head:

go test ./pkg/controller/v1beta1/inferenceservice/components -run TestMergeRuntimeArgumentsOverride -count=1

The DP override follows the existing overrideParam behavior used for TP/PP, and the added tests cover long flags, SGLang size flags, command-based shorthand flags, and single-dash aliases. PR checks are green as of this review.

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

Labels

controller Controller changes inferenceservice InferenceService controller changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants