Repository navigation
[llm_d] Work on P/D CPT - #126
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughTemplate-based deployment profiles, P/D rendering, scheduler and Kueue configuration, Guidellm workloads, preset inheritance updates, generated-manifest references, and integration tests are added or updated for llm-d orchestration. Changesllm-d deployment orchestration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Preset
participant RuntimeConfig
participant Renderer
participant Scheduler
participant KServeManifest
Preset->>RuntimeConfig: resolve deployment profile
RuntimeConfig->>Scheduler: inject EPP configuration
RuntimeConfig->>Renderer: provide resolved profile
Renderer->>KServeManifest: render standard or P/D service
Renderer->>KServeManifest: apply Kueue metadata
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/test fournos llm_d smoke |
|
/test fournos llm_d smoke |
🛑 Execution of
|
|
/test fournos llm_d smoke |
🔴 Execution of
|
🔴 Submission of
|
|
/test fournos llm_d smoke |
🔴 Execution of
|
🔴 Submission of
|
|
/test fournos llm_d smoke |
🔴 Execution of
|
🔴 Submission of
|
a94115b to
e503280
Compare
|
/test fournos llm_d smoke |
🟢 Execution of
|
🟢 Submission of
|
|
/test fournos llm_d smoke |
|
/test fournos llm_d smoke |
🔴 Submission of
|
|
/test fournos llm_d smoke |
🔴 Submission of
|
76f89ea to
94d503b
Compare
|
/test fournos llm_d smoke |
🟢 Execution of
|
🟢 Submission of
|
64caa2d to
9245d42
Compare
…e_is_managed flag
| vllm_default_args = _build_vllm_args( | ||
| config.project.get_config("deployments.defaults.vllm_extra.args") | ||
| ) | ||
| vllm_deploy_args = _build_vllm_args(deployment_profile.get("vllm_extra.args", {})) |
There was a problem hiding this comment.
Does this dict.get() work? I think the deployment profile is a nested dict: {"vllm_extra": {"args": [...]}} so I'm not sure if this is working.
There was a problem hiding this comment.
good catch, it "works" (doesn't crash), but doesn't do anything useful 😅
actually, it went unnoticed because the default args are already applied earlier in the runtime config, so this call is redundant
fixed by removing deployments.defaults.vllm_extra.args and properly fetching vllm_extra.args
…e already applied in runtime_config.get_deployment_profile()
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: albertoperdomo2 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary by CodeRabbit