Skip to content

fix(engine): support validated TP x PP topologies - #29

Draft
ShuhaoZhangTony wants to merge 1 commit into
mainfrom
codex/pipeline-parallel-topology
Draft

fix(engine): support validated TP x PP topologies#29
ShuhaoZhangTony wants to merge 1 commit into
mainfrom
codex/pipeline-parallel-topology

Conversation

@ShuhaoZhangTony

Copy link
Copy Markdown
Contributor

Summary

  • replace the incorrect TP == device count launcher gate with TP x PP == device count
  • validate positive TP/PP sizes and reject conflicting pipeline CLI configuration
  • inject --pipeline-parallel-size consistently and record the managed topology

The previous gate made a valid four-card PP2 x TP2 Pipeline Microbatch qualification impossible. This change does not introduce TP1/eager/legacy fallback.

Tests

  • bash -n tools/run_vllm_engine.sh: passed
  • python -m pytest -q tests/test_systemd_service_scripts.py: 16 passed
  • Ruff and git diff --check: passed
  • Qwen3.8-27B real NPU0-3 PP2 x TP2 FULL_DECODE_ONLY graph qualification completed through the corrected topology path

Qualification boundary

The Mod was runtime-effective and functionally correct, but matched performance regressed; the tested configuration remains unsupported pending fresh calibration.

Contribution disclosure

AI assistance was used to diagnose the topology gate, edit the launcher, and run tests. Human line-by-line review is required before merge.

Replace the TP-equals-device-count gate with a TP-times-PP invariant and keep pipeline-parallel CLI arguments consistent with the managed environment.

Generated with AI assistance; reviewed and tested by the committer.
@ShuhaoZhangTony

Copy link
Copy Markdown
Contributor Author

CI triage: the scoped launcher checks are green (lint, install-linux-cpu, validate-linux-ascend) and the local launcher test file passes 16/16. The full test failure is in four unrelated agentic-workflow knowledge/recommended-reading assertions; frontend is one pre-existing dark-theme screenshot drift. The current main branch is already red (for example run 33836060735). No application/frontend code is changed by this PR, so those baseline failures are intentionally not patched here.

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.

1 participant