Skip to content

[None][docs] drop ghost enable_overlap_scheduler from trtllm-serve Slurm example - #18961

Open
imitater-dou wants to merge 1 commit into
NVIDIA:mainfrom
imitater-dou:docs-drop-ghost-enable-overlap-scheduler-serve-rst
Open

[None][docs] drop ghost enable_overlap_scheduler from trtllm-serve Slurm example#18961
imitater-dou wants to merge 1 commit into
NVIDIA:mainfrom
imitater-dou:docs-drop-ghost-enable-overlap-scheduler-serve-rst

Conversation

@imitater-dou

@imitater-dou imitater-dou commented Sep 9, 2026

Copy link
Copy Markdown

Summary

  • The Multi-node Slurm sample in docs/source/commands/trtllm-serve/trtllm-serve.rst wrote pytorch_backend_config.enable_overlap_scheduler: true.
  • That nested key is not a TorchLlmArgs field; the live knob is top-level disable_overlap_scheduler (default false, i.e. overlap on). See docs/source/features/overlap-scheduler.md.
  • Keep enable_attention_dp: true in the sample YAML; drop the ghost overlap stanza.

Test plan

  • Confirmed enable_overlap_scheduler has 0 Python hits on main; only this rst referenced it
  • Confirmed disable_overlap_scheduler is the documented/live field in llm_args.py and feature docs
  • Diff is a one-line echo rewrite in the Slurm sample

Dev Engineer Review

The Multi-node Slurm example removes the unsupported pytorch_backend_config.enable_overlap_scheduler setting. It retains enable_attention_dp: true and relies on the supported default that enables overlap scheduling.

QA Engineer Review

No test changes.

Per-File QA Perspective

  • docs/source/commands/trtllm-serve/trtllm-serve.rst: Verify that the example generates valid configuration and preserves attention data parallelism and default overlap scheduling behavior.

…urm example

The Multi-node Slurm sample nested pytorch_backend_config.enable_overlap_scheduler,
which is not a TorchLlmArgs field. Overlap is on by default; live knob is top-level
disable_overlap_scheduler (see docs/source/features/overlap-scheduler.md).

Keep enable_attention_dp: true; drop the ghost overlap stanza.

Signed-off-by: ADou <ikun3.1415927@gmail.com>
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5d681eb7-dcd8-4b86-8964-9b4a1841e74d

📥 Commits

Reviewing files that changed from the base of the PR and between 96a25c4 and 2d73128.

📒 Files selected for processing (1)
  • docs/source/commands/trtllm-serve/trtllm-serve.rst

Included review availability: Your plan provides up to 12 included reviews per hour; 7 remain after this review.


Walkthrough

The Slurm multi-node serving example now writes only enable_attention_dp: true to config.yml. It no longer writes pytorch_backend_config.enable_overlap_scheduler: true.

Changes

Slurm serving configuration

Layer / File(s) Summary
Update Slurm configuration example
docs/source/commands/trtllm-serve/trtllm-serve.rst
The example retains enable_attention_dp: true and removes pytorch_backend_config.enable_overlap_scheduler: true.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 2d731

The Slurm serving example no longer documents an unsupported overlap-scheduler option and continues to enable attention data parallelism. No current merge-readiness risk remains.

Suggested reviewers: bowenfu

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the obsolete configuration, the replacement behavior, and the validation performed. It includes summary and test-plan content, although it does not reproduce the optio…
Title check ✅ Passed The title clearly identifies the documentation change and the removal of the obsolete enable_overlap_scheduler setting from the trtllm-serve Slurm example. It follows the required [None][type] format …
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

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.

2 participants