[Refactor][Device][7/N] Migrate MoE compilation and sampling to hardware profiles - #15478
Conversation
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request serves as the final installment (7/7) of the Device HAL refactor series. It completes the decoupling of shared business logic from specific Ascend device identities by transitioning MoE compilation, routing, and sampling to a hardware capability and policy-based architecture. This change improves maintainability and simplifies the integration of future hardware targets by ensuring that consumers of shared logic rely on explicit capability checks rather than branching on device identity. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Ops][Misc] Refactor hardware capability and MoE communication policy checksSuggested PR Summary:
### What this PR does / why we need it?
This PR refactors the hardware capability and MoE communication policy checks across the codebase. Instead of directly checking `AscendDeviceType` (e.g., `A2`, `A3`, `A5`, `310P`), the code now queries the active `HardwareProfile` for specific `HardwareCapability` flags and `MoECommPolicy` configurations. This decouples device-specific logic from operator and communication implementations, making the codebase more maintainable and extensible for future hardware generations.
Additionally, a review comment suggests optimizing the hot path in `select_moe_comm_method` by moving the `selector_by_policy` dictionary to the module level as a constant to avoid recreation overhead on every forward pass.
### Does this PR introduce _any_ user-facing change?
No. This is an internal refactoring of hardware profile and capability checks.
### How was this patch tested?
Existing unit tests in `tests/ut/device/test_hardware_profile.py`, `tests/ut/ops/a2/test_token_dispatcher.py`, `tests/ut/ops/test_moe_mlp.py`, and `tests/ut/test_ascend_forward_context.py` were updated to mock the hardware profile and passed successfully.|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
8e69b3c to
da260e0
Compare
…are profiles Signed-off-by: frost_mourne <2906339855@qq.com>
Signed-off-by: frost_mourne <2906339855@qq.com>
Signed-off-by: frost_mourne <2906339855@qq.com>
da260e0 to
3e8c789
Compare
…are profiles (vllm-project#15478) ## What - add the final MoE/compilation/sampling hardware capabilities and a `MoECommPolicy` - migrate MoE communication selection, graph fusion gates, routing, shared experts, token dispatch, SwiGLU-OAI MX quant, and top-k/top-p sampling away from direct device identity checks - preserve the latest upstream Situ, hierarchical communication, SFA, DSV4, sampler, and CANN MegaMoe paths - model the new A3-only CANN MegaMoe consumer with `HardwareCapability.CANN_MEGAMOE` and update the exact capability matrix ## Why Shared business logic should consume hardware capabilities and implementation policies instead of branching on Ascend device identities. This keeps hardware detection/profile registration as the single identity boundary and makes the final consumers explicit. This is a refactor only; there is no user-visible behavior change. ## Series Device HAL / Hardware Profile refactor 7/N. Depends on merged vllm-project#15407 (`d4ebe8a0e0fdf56135507259f90f80a45f736cab`) and is rebased onto current `main` (`30f54b5c341f015bcc124022e62156d3e5ef5919`). This base includes the GLM5.3 documentation-link fix from vllm-project#15508 and the MegaMoe prefill update from vllm-project#14439. ## Testing - `ruff check` on all 14 changed Python files - `ruff format --check` on all 14 changed Python files - `python3 -m compileall -q` on all 14 changed Python files - `git diff --check upstream/main..HEAD` - production-code scan for newly introduced direct device-identity branches - E2E run https://github.com/vllm-project/vllm-ascend/actions/runs/33579196584: 23 successful jobs, 4 skipped, zero failures; CPU, all selected NPU jobs, 310P, upstream, pre-commit, DCO, Read the Docs, and `ci-gate` succeeded The local static checks above passed. The development server does not provide the complete pytest/mypy/vLLM environment, so targeted unit tests and Python 3.10/3.11/3.12 mypy comparison are covered by the successful CI run and are not claimed as local results. --------- Signed-off-by: frost_mourne <2906339855@qq.com>
…are profiles (vllm-project#15478) ## What - add the final MoE/compilation/sampling hardware capabilities and a `MoECommPolicy` - migrate MoE communication selection, graph fusion gates, routing, shared experts, token dispatch, SwiGLU-OAI MX quant, and top-k/top-p sampling away from direct device identity checks - preserve the latest upstream Situ, hierarchical communication, SFA, DSV4, sampler, and CANN MegaMoe paths - model the new A3-only CANN MegaMoe consumer with `HardwareCapability.CANN_MEGAMOE` and update the exact capability matrix ## Why Shared business logic should consume hardware capabilities and implementation policies instead of branching on Ascend device identities. This keeps hardware detection/profile registration as the single identity boundary and makes the final consumers explicit. This is a refactor only; there is no user-visible behavior change. ## Series Device HAL / Hardware Profile refactor 7/N. Depends on merged vllm-project#15407 (`d4ebe8a0e0fdf56135507259f90f80a45f736cab`) and is rebased onto current `main` (`30f54b5c341f015bcc124022e62156d3e5ef5919`). This base includes the GLM5.3 documentation-link fix from vllm-project#15508 and the MegaMoe prefill update from vllm-project#14439. ## Testing - `ruff check` on all 14 changed Python files - `ruff format --check` on all 14 changed Python files - `python3 -m compileall -q` on all 14 changed Python files - `git diff --check upstream/main..HEAD` - production-code scan for newly introduced direct device-identity branches - E2E run https://github.com/vllm-project/vllm-ascend/actions/runs/33579196584: 23 successful jobs, 4 skipped, zero failures; CPU, all selected NPU jobs, 310P, upstream, pre-commit, DCO, Read the Docs, and `ci-gate` succeeded The local static checks above passed. The development server does not provide the complete pytest/mypy/vLLM environment, so targeted unit tests and Python 3.10/3.11/3.12 mypy comparison are covered by the successful CI run and are not claimed as local results. --------- Signed-off-by: frost_mourne <2906339855@qq.com>
#16803) ### What this PR does / why we need it? Follow-up to the `[ Refactor ][ Device ][ x/N ] ... to hardware profiles` series (#14076, #15256, #15376, #15407, #15478). That series migrated most `is_ 310p() ` call sites to semantic hardware-profile capabilities, but three call sites in the v2 worker runtime path were left behind, and the ` is _310p()` compatibility helper itself was kept alive. This PR finishes the migration and removes the helper. Residual call sites removed: - `vllm_ ascend/patch/worker/patch _v2/patch_ block _table.py` — selects `Ascend310PBlockTables` on 310P - `vllm_ ascend/worker/v2/model _states/ __init__ .py` — selects the Triton-free 310P `ModelState` (2 sites) - `vllm_ ascend/patch/platform/patch _use_ v2 _model_ runner.py` — 310P skips the upstream v2 model runner validation ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - vLLM main: vllm-project/vllm@84030bb Signed-off-by: spoon1116 <1522707055@qq.com>
vllm-project#16803) ### What this PR does / why we need it? Follow-up to the `[ Refactor ][ Device ][ x/N ] ... to hardware profiles` series (vllm-project#14076, vllm-project#15256, vllm-project#15376, vllm-project#15407, vllm-project#15478). That series migrated most `is_ 310p() ` call sites to semantic hardware-profile capabilities, but three call sites in the v2 worker runtime path were left behind, and the ` is _310p()` compatibility helper itself was kept alive. This PR finishes the migration and removes the helper. Residual call sites removed: - `vllm_ ascend/patch/worker/patch _v2/patch_ block _table.py` — selects `Ascend310PBlockTables` on 310P - `vllm_ ascend/worker/v2/model _states/ __init__ .py` — selects the Triton-free 310P `ModelState` (2 sites) - `vllm_ ascend/patch/platform/patch _use_ v2 _model_ runner.py` — 310P skips the upstream v2 model runner validation ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - vLLM main: vllm-project/vllm@84030bb Signed-off-by: spoon1116 <1522707055@qq.com>
vllm-project#16803) ### What this PR does / why we need it? Follow-up to the `[ Refactor ][ Device ][ x/N ] ... to hardware profiles` series (vllm-project#14076, vllm-project#15256, vllm-project#15376, vllm-project#15407, vllm-project#15478). That series migrated most `is_ 310p() ` call sites to semantic hardware-profile capabilities, but three call sites in the v2 worker runtime path were left behind, and the ` is _310p()` compatibility helper itself was kept alive. This PR finishes the migration and removes the helper. Residual call sites removed: - `vllm_ ascend/patch/worker/patch _v2/patch_ block _table.py` — selects `Ascend310PBlockTables` on 310P - `vllm_ ascend/worker/v2/model _states/ __init__ .py` — selects the Triton-free 310P `ModelState` (2 sites) - `vllm_ ascend/patch/platform/patch _use_ v2 _model_ runner.py` — 310P skips the upstream v2 model runner validation ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - vLLM main: vllm-project/vllm@84030bb Signed-off-by: spoon1116 <1522707055@qq.com>
vllm-project#16803) ### What this PR does / why we need it? Follow-up to the `[ Refactor ][ Device ][ x/N ] ... to hardware profiles` series (vllm-project#14076, vllm-project#15256, vllm-project#15376, vllm-project#15407, vllm-project#15478). That series migrated most `is_ 310p() ` call sites to semantic hardware-profile capabilities, but three call sites in the v2 worker runtime path were left behind, and the ` is _310p()` compatibility helper itself was kept alive. This PR finishes the migration and removes the helper. Residual call sites removed: - `vllm_ ascend/patch/worker/patch _v2/patch_ block _table.py` — selects `Ascend310PBlockTables` on 310P - `vllm_ ascend/worker/v2/model _states/ __init__ .py` — selects the Triton-free 310P `ModelState` (2 sites) - `vllm_ ascend/patch/platform/patch _use_ v2 _model_ runner.py` — 310P skips the upstream v2 model runner validation ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - vLLM main: vllm-project/vllm@84030bb Signed-off-by: spoon1116 <1522707055@qq.com>
What
MoECommPolicyHardwareCapability.CANN_MEGAMOEand update the exact capability matrixWhy
Shared business logic should consume hardware capabilities and implementation policies instead of branching on Ascend device identities. This keeps hardware detection/profile registration as the single identity boundary and makes the final consumers explicit.
This is a refactor only; there is no user-visible behavior change.
Series
Device HAL / Hardware Profile refactor 7/N.
Depends on merged #15407 (
d4ebe8a0e0fdf56135507259f90f80a45f736cab) and is rebased onto currentmain(30f54b5c341f015bcc124022e62156d3e5ef5919). This base includes the GLM5.3 documentation-link fix from #15508 and the MegaMoe prefill update from #14439.Testing
ruff checkon all 14 changed Python filesruff format --checkon all 14 changed Python filespython3 -m compileall -qon all 14 changed Python filesgit diff --check upstream/main..HEADci-gatesucceededThe local static checks above passed. The development server does not provide the complete pytest/mypy/vLLM environment, so targeted unit tests and Python 3.10/3.11/3.12 mypy comparison are covered by the successful CI run and are not claimed as local results.