Repository navigation
[FIX_FOR_VLLM_CUSTOM=9a70c233cd5d3ef3b14d79c582a6f9ddfa03cf80] forward skip_padding kwarg, drop stale manager.touch() (+3 more) - #1806
Merged
iboiko-habana merged 5 commits intoSep 22, 2026
Conversation
pawel-olejniczak
requested review from
PatrykWo,
adobrzyn,
afierka-intel,
iboiko-habana,
jbyczkow,
mgawarkiewicz-intel,
michalkuligowski and
xuechendi
as code owners
September 21, 2026 11:21
pawel-olejniczak
had a problem deploying
to
pre-merge-approval
September 21, 2026 11:21 — with
GitHub Actions
Error
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: None
What changed in this PR
This PR updates vLLM-Gaudi compatibility with newer upstream vLLM APIs and removes obsolete offloading scheduler assertions.
Changes:
- Adds
skip_paddingsupport to the HPU fused MoE router factory. - Avoids assigning to the new read-only
is_sleepingproperty and addsrelease_kv_cache_memorydelegation. - Updates the offloading connector test to reflect removal of
manager.touch().
| File | Description |
|---|---|
vllm_gaudi/v1/engine/core_patch.py |
Handles executor sleep-state reset across old and new vLLM APIs. |
vllm_gaudi/ops/hpu_fused_moe.py |
Accepts and forwards the upstream skip_padding router argument. |
vllm_gaudi/entrypoints/openai/multi_model_api_server.py |
Delegates KV-cache memory release to the wrapped engine. |
tests/unit_tests/kv_offload/offloading_connector/test_scheduler.py |
Removes assertions for the deleted manager.touch() behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Root cause: upstream vllm#56079 added a `skip_padding: bool = False` kwarg to create_fused_moe_router()/GroupedTopKRouter to invalidate sequence-parallel padding rows in grouped MoE routing. The vllm-gaudi override mirrors this factory's signature and did not accept the new kwarg, so every call site now raises: TypeError: create_fused_moe_router() got an unexpected keyword argument 'skip_padding' Upstream: vllm-project/vllm#56079 Fix: accept skip_padding on the override (signature parity) and forward it to GroupedTopKRouter; the guard it enables only fires under VLLM_MOE_SKIP_PADDING + DeepEP-v2-kernel forward context, which HPU never exercises, so this is a no-op on HPU beyond restoring the call signature. Signed-off-by: Paweł Olejniczak <pawelx.olejniczak@intel.com>
Root cause: upstream vllm#51787 ("Track cache recency once per
request") removed OffloadingConnectorScheduler._touch and every
manager.touch() call site, replacing per-call cache-recency tracking
with req_context.set_offload_key_position() tracked once per request.
vllm-gaudi's vendored copy of the unit test still asserted the old
manager.touch() calls, so it now fails:
AssertionError: Expected 'touch' to have been called.
Upstream: vllm-project/vllm#51787
Fix: mirror upstream's own post-#51787 test update (its analogous
tests/v1/kv_connector/unit/offloading_connector/test_scheduler.py) by
dropping the removed-call assertions and asserting touch is no longer
called for the same scenario.
Signed-off-by: Paweł Olejniczak <pawelx.olejniczak@intel.com>
Root cause: upstream vllm#44890 added a new abstract method release_kv_cache_memory to EngineClient; MultiModelEngineClient never implemented it, so instantiation raised TypeError. Upstream: vllm-project/vllm#44890 Fix: delegate to the wrapped AsyncLLM instance, mirroring the existing sleep/wake_up passthrough pattern. Signed-off-by: Paweł Olejniczak <pawelx.olejniczak@intel.com>
Root cause: upstream vllm#44890 (same PR as the MultiModelEngineClient abstract-method break) turned Executor.is_sleeping into a read-only property computed from sleeping_tags. gaudi_reconfigure_engine's _reset_executor_sleep_state still assigned model_executor.is_sleeping directly, raising AttributeError: property 'is_sleeping' of 'UniProcExecutor' object has no setter during online model swap. Upstream: vllm-project/vllm#44890 Fix: feature-detect whether is_sleeping is a class-level property before assigning it; clearing sleeping_tags already makes the property read False on the new executor. Signed-off-by: Paweł Olejniczak <pawelx.olejniczak@intel.com>
pawel-olejniczak
force-pushed
the
fix/batch-20260921t084517z
branch
from
September 21, 2026 13:54
5a7a48c to
198d7e9
Compare
pawel-olejniczak
deployed
to
pre-merge-approval
September 21, 2026 13:54 — with
GitHub Actions
Active
Root cause: upstream vllm PR 53555 ("[LoRA] Support modules_to_save for
sequence classification") added a new abstract method
apply_lora_full_linear to PunicaWrapperABC and implemented it for CPU,
GPU and XPU. PunicaWrapperHPU never implemented it, so it is no longer a
concrete class and get_punica_wrapper() raises
TypeError: Can't instantiate abstract class PunicaWrapperHPU without an
implementation for abstract method 'apply_lora_full_linear'
while loading any LoRA model on HPU, which the engine surfaces as the
generic "Engine core initialization failed".
Upstream: vllm-project/vllm#53555
Fix: implement the hook with plain torch ops, mirroring the CPU and GPU
versions: route rows through the prompt-level sampler_indices mapping and
hand off to the inherited _select_full_linear_output helper for the bias
and module-enabled selection. Multiplying against every adapter and
gathering the routed row afterwards keeps the weight operand statically
shaped for HPU graph compilation. The new method only references
_select_full_linear_output at call time, and its only caller
(ClassificationHeadWithLoRA) postdates PR 53555, so the change stays
compatible with older VLLM_STABLE_COMMIT builds.
Signed-off-by: Paweł Olejniczak <pawelx.olejniczak@intel.com>
pawel-olejniczak
deployed
to
pre-merge-approval
September 22, 2026 10:59 — with
GitHub Actions
Active
Contributor
✅ CI PassedAll checks passed successfully against the following vllm commit: |
iboiko-habana
approved these changes
Sep 22, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR consolidates 5 hourly-CI fixes against vllm@
9a70c233cd5d3ef3b14d79c582a6f9ddfa03cf80.Bug 1: forward skip_padding kwarg through HPU MoE router factory override
Root cause
upstream vllm#56079 added a
skip_padding: bool = Falsekwarg to create_fused_moe_router()/GroupedTopKRouter to invalidate sequence-parallel padding rows in grouped MoE routing. The vllm-gaudi override mirrors this factory's signature and did not accept the new kwarg, so every call site now raises: TypeError: create_fused_moe_router() got an unexpected keyword argument 'skip_padding'Culprit
Regression introduced by PR #56079.
Fix
accept skip_padding on the override (signature parity) and forward it to GroupedTopKRouter; the guard it enables only fires under VLLM_MOE_SKIP_PADDING + DeepEP-v2-kernel forward context, which HPU never exercises, so this is a no-op on HPU beyond restoring the call signature.
Bug 2: drop stale manager.touch() assertions in offloading connector test
Root cause
upstream vllm#51787 ("Track cache recency once per request") removed OffloadingConnectorScheduler._touch and every manager.touch() call site, replacing per-call cache-recency tracking with req_context.set_offload_key_position() tracked once per request. vllm-gaudi's vendored copy of the unit test still asserted the old manager.touch() calls, so it now fails: AssertionError: Expected 'touch' to have been called.
Culprit
Probable culprit: PR #51787 - pinned by symbol archaeology, bisect not run.
Candidate range: compare.
Fix
mirror upstream's own post-#51787 test update (its analogous tests/v1/kv_connector/unit/offloading_connector/test_scheduler.py) by dropping the removed-call assertions and asserting touch is no longer called for the same scenario.
Bug 3: add missing release_kv_cache_memory to MultiModelEngineClient
Root cause
upstream vllm#44890 added a new abstract method release_kv_cache_memory to EngineClient; MultiModelEngineClient never implemented it, so instantiation raised TypeError.
Culprit
Probable culprit: PR #44890 - pinned by symbol archaeology, bisect not run.
Candidate range: compare.
Fix
delegate to the wrapped AsyncLLM instance, mirroring the existing sleep/wake_up passthrough pattern.
Bug 4: skip is_sleeping assignment on read-only Executor property
Root cause
upstream vllm#44890 (same PR as the MultiModelEngineClient abstract-method break) turned Executor.is_sleeping into a read-only property computed from sleeping_tags. gaudi_reconfigure_engine's _reset_executor_sleep_state still assigned model_executor.is_sleeping directly, raising AttributeError: property 'is_sleeping' of 'UniProcExecutor' object has no setter during online model swap.
Culprit
Probable culprit: PR #44890 - pinned by symbol archaeology, bisect not run.
Candidate range: compare.
Fix
feature-detect whether is_sleeping is a class-level property before assigning it; clearing sleeping_tags already makes the property read False on the new executor.
Bug 5: implement apply_lora_full_linear on PunicaWrapperHPU
Root cause
upstream vllm PR 53555 ("[LoRA] Support modules_to_save for sequence classification") added a new abstract method apply_lora_full_linear to PunicaWrapperABC and implemented it for CPU, GPU and XPU. PunicaWrapperHPU never implemented it, so it is no longer a concrete class and get_punica_wrapper() raises TypeError: Can't instantiate abstract class PunicaWrapperHPU without an implementation for abstract method 'apply_lora_full_linear' while loading any LoRA model on HPU, which the engine surfaces as the generic "Engine core initialization failed".
Culprit
Probable culprit: PR #53555 - pinned by symbol archaeology, bisect not run.
Candidate range: compare.
Fix
implement the hook with plain torch ops, mirroring the CPU and GPU versions: route rows through the prompt-level sampler_indices mapping and hand off to the inherited _select_full_linear_output helper for the bias and module-enabled selection. Multiplying against every adapter and gathering the routed row afterwards keeps the weight operand statically shaped for HPU graph compilation. The new method only references _select_full_linear_output at call time, and its only caller (ClassificationHeadWithLoRA) postdates PR 53555, so the change stays compatible with older VLLM_STABLE_COMMIT builds.