Skip to content

fix(slurm): default pipeline parallelism to one - #1116

Merged
nabinchha merged 3 commits into
feat/slurm-executionfrom
codex/slurm-parallelism-defaults
Oct 30, 2026
Merged

nabinchha merged 3 commits into
feat/slurm-executionfrom
codex/slurm-parallelism-defaults

Conversation

@nabinchha

@nabinchha nabinchha commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

📋 Summary

Default pipeline parallelism to one stage, independently of the number of nodes per replica. A TP16 deployment across two eight-GPU nodes now works with PP omitted; additional pipeline stages must be requested explicitly. Expert parallelism remains disabled by default and opt-in for compatible MoE models.

Earlier persisted runs are not migrated; compatibility with their prior topology and serialization semantics is intentionally outside this change.

🔗 Related Issue

Related to #850; follows #1112.

🔄 Changes

  • Use pipeline_parallel: 1 in authored configuration and topology derivation, removing the node-count fallback.
  • Cover omitted, disabled, and enabled EP across single-node and multi-node TP/PP layouts, including TP16 on eight-GPU nodes with PP and EP omitted.
  • Verify benchmark child plans preserve default PP1 or explicit PP2 and replica counts when node placement changes.
  • Provide a dedicated TP8 benchmark base run and compile every child plan from both the guide and shipped examples in regression tests. Document the two-node/eight-GPU requirement and per-child validation.
  • Make pipeline fixtures request PP2 explicitly and refresh their derived checksums and benchmark identities.
  • Clarify TP, PP, independent replicas, GPU arithmetic, and optional EP in the model-agnostic documentation. Include a two-node/eight-GPU example and update profile and benchmark guidance.

🧪 Testing

  • Full Slurm suite: 2,127 passed, 1 platform-specific skip.
  • Slurm-wide Ruff lint and formatting checks.
  • Fern validation: zero errors; one authentication warning for the redirects check.
  • Internal links across 101 Fern pages and all eight documented topology-table calculations.
  • Live cluster rerun: N/A; this changes configuration defaults, with generated launch commands covered by integration tests.

✅ Checklist

  • Follows commit message conventions
  • Commits are signed off (DCO)
  • User-facing Slurm documentation updated
  • Architecture docs: N/A; the serving architecture is unchanged.

Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
@nabinchha
nabinchha requested a review from a team as a code owner October 29, 2026 21:53
@github-actions

Copy link
Copy Markdown
Contributor

Fern preview: https://nvidia-preview-pr-1116.docs.buildwithfern.com/nemo/datadesigner

Fern previews include the docs-website version archive with PR changes synced into latest. Notebook tutorials are rendered without execution outputs in previews.

@greptile-apps

greptile-apps Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge within its stated saved-run compatibility limits.

Summary

This PR defaults pipeline_parallel to one, independently of node count.

  • Slurm runs keep one pipeline stage unless you ask for more.
  • Benchmark examples use a TP8 base run for node-placement comparisons.

nabinchha explicitly accepted saved-run incompatibility because preserving earlier topology and serialization behavior is outside this change. The prior unnumbered finding is not reposted.

Reviews (3) · Last reviewed commit: "fix(slurm): use a compatible benchmark b..." · Reviewed by Greptile

Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>

@andreatnvidia andreatnvidia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please address the benchmark example issue before merging.

Signed-off-by: Nabin Mulepati <nmulepati@nvidia.com>
@nabinchha
nabinchha merged commit 678a382 into feat/slurm-execution Oct 30, 2026
10 checks passed
@nabinchha
nabinchha deleted the codex/slurm-parallelism-defaults branch October 30, 2026 17:31
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