Skip to content

[Platform] Make forward context manager pluggable for other device#29388

Closed
shen-shanshan wants to merge 4 commits intovllm-project:mainfrom
shen-shanshan:platform
Closed

[Platform] Make forward context manager pluggable for other device#29388
shen-shanshan wants to merge 4 commits intovllm-project:mainfrom
shen-shanshan:platform

Conversation

@shen-shanshan
Copy link
Copy Markdown
Contributor

@shen-shanshan shen-shanshan commented Nov 25, 2025 •
edited by github-actions bot
Loading

Purpose

Currently, set_forward_context is directly used in some modeling files, such as Qwen2.5-VL (when process image/video input with ViT).

We want to use some custom forward context here (without modifying the modeling files in the plugin), such as https://github.com/vllm-project/vllm-ascend/blob/main/vllm_ascend/ascend_forward_context.py#L58.

Thus, this PR has made forward context manager pluggable for other device.

Test Plan

Test Result

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

@shen-shanshan
make forward context manager pluggable
27780a1 
Signed-off-by: shen-shanshan <467638484@qq.com>
@shen-shanshan
update
f4fe99b 
Signed-off-by: shen-shanshan <467638484@qq.com>
@mergify mergify bot added the qwen Related to Qwen models label Nov 25, 2025
@shen-shanshan shen-shanshan mentioned this pull request Nov 25, 2025
Closed
17 tasks
gemini-code-assist[bot]
gemini-code-assist bot reviewed Nov 25, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the usage of set_forward_context to be pluggable, allowing different platforms to provide their own forward context manager. This is achieved by introducing a get_forward_context_manager method in the Platform interface. The implementation is sound and improves modularity. My only feedback is on a minor code duplication issue in qwen2_5_vl.py where current_platform is imported locally in two separate methods. I've suggested a refactoring to improve maintainability.

Comment thread vllm/model_executor/models/qwen2_5_vl.py Outdated
@shen-shanshan
minor fix
69f01ee 
Signed-off-by: shen-shanshan <467638484@qq.com>
@shen-shanshan shen-shanshan mentioned this pull request Nov 25, 2025
Merged
13 tasks
DarkLight1337
DarkLight1337 reviewed Nov 25, 2025
View reviewed changes
Comment thread vllm/model_executor/models/qwen2_5_vl.py Outdated
self.language_model.make_empty_intermediate_tensors
)

from vllm.platforms import current_platform
Copy link
Copy Markdown
Member

@DarkLight1337 DarkLight1337 Nov 25, 2025

Choose a reason for hiding this comment

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

I think there is no need to import this lazily, just import it from top level

Copy link
Copy Markdown
Contributor Author

@shen-shanshan shen-shanshan Nov 26, 2025

Choose a reason for hiding this comment

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

I think there is no need to import this lazily, just import it from top level

done.

ProExpertProg
ProExpertProg reviewed Nov 25, 2025
View reviewed changes
Comment thread vllm/platforms/interface.py Outdated
return max_model_len

@classmethod
def get_forward_context_manager(cls):
Copy link
Copy Markdown
Collaborator

@ProExpertProg ProExpertProg Nov 25, 2025

Choose a reason for hiding this comment

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

Can you add a type hint here?

Copy link
Copy Markdown
Contributor Author

@shen-shanshan shen-shanshan Nov 26, 2025

Choose a reason for hiding this comment

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

Can you add a type hint here?

done.

@shen-shanshan
minor fix
aa2947c 
Signed-off-by: shen-shanshan <467638484@qq.com>
@shen-shanshan
Copy link
Copy Markdown
Contributor Author

shen-shanshan commented Nov 28, 2025 •
edited
Loading

After offline discussing with @wangxiyuan , this PR is not needed and we decided to directly refactor our set_ascend_forward_context(). Thanks for reviewing~

wangxiyuan pushed a commit to vllm-project/vllm-ascend that referenced this pull request Nov 28, 2025
@shen-shanshan
[MM][Model][Perf] Remove Qwen2.5-VL modeling files and add patch for …
e52ebf8 
…VisionAttention (#4349)

### What this PR does / why we need it?

- [x] Patch `Qwen2_5_VisionAttention` with
`AscendQwen2_5_VisionAttention`.
- [x] Replace `AscendQwen2_5_VisionTransformer` with
`Qwen2_5_VisionTransformer` in vllm.
- [x] Move padding logic (q/k/v and cos/sin) before FA to `forward()` of
`Qwen2_5_VisionAttention`.
- [x] Covert `cu_seqlens` in `Qwen2_5_VisionAttention` from cumulative
form to intervals and move it to cpu (compatible for npu FA).
- [x] Remove Qwen2.5-VL modeling files.
- [x] Remove Qwen2.5-VL (without padding) modeling files.
- [x] Remove related UT.
- [x] Make `set_forward_context` pluggable when getting MM embedding.
Find more details at vllm-project/vllm#29388.
- [x] Simplify padding logic for FA.
- [x] Add patch for vllm-project/vllm#28798.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

- [x] Functional test (eager mode)
- [x] Functional test (graph mode)
- [x] Benchmark


- vLLM version: v0.11.2

---------

Signed-off-by: shen-shanshan <467638484@qq.com>
ChenCangtao pushed a commit to ChenCangtao/vllm-ascend that referenced this pull request Dec 3, 2025
@shen-shanshan
[MM][Model][Perf] Remove Qwen2.5-VL modeling files and add patch for …
ff355f4 
…VisionAttention (vllm-project#4349)

### What this PR does / why we need it?

- [x] Patch `Qwen2_5_VisionAttention` with
`AscendQwen2_5_VisionAttention`.
- [x] Replace `AscendQwen2_5_VisionTransformer` with
`Qwen2_5_VisionTransformer` in vllm.
- [x] Move padding logic (q/k/v and cos/sin) before FA to `forward()` of
`Qwen2_5_VisionAttention`.
- [x] Covert `cu_seqlens` in `Qwen2_5_VisionAttention` from cumulative
form to intervals and move it to cpu (compatible for npu FA).
- [x] Remove Qwen2.5-VL modeling files.
- [x] Remove Qwen2.5-VL (without padding) modeling files.
- [x] Remove related UT.
- [x] Make `set_forward_context` pluggable when getting MM embedding.
Find more details at vllm-project/vllm#29388.
- [x] Simplify padding logic for FA.
- [x] Add patch for vllm-project/vllm#28798.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

- [x] Functional test (eager mode)
- [x] Functional test (graph mode)
- [x] Benchmark

- vLLM version: v0.11.2

---------

Signed-off-by: shen-shanshan <467638484@qq.com>
Mercykid-bash pushed a commit to Mercykid-bash/vllm-ascend that referenced this pull request Dec 4, 2025
@shen-shanshan @mercykid
[MM][Model][Perf] Remove Qwen2.5-VL modeling files and add patch for …
9362171 
…VisionAttention (vllm-project#4349)

### What this PR does / why we need it?

- [x] Patch `Qwen2_5_VisionAttention` with
`AscendQwen2_5_VisionAttention`.
- [x] Replace `AscendQwen2_5_VisionTransformer` with
`Qwen2_5_VisionTransformer` in vllm.
- [x] Move padding logic (q/k/v and cos/sin) before FA to `forward()` of
`Qwen2_5_VisionAttention`.
- [x] Covert `cu_seqlens` in `Qwen2_5_VisionAttention` from cumulative
form to intervals and move it to cpu (compatible for npu FA).
- [x] Remove Qwen2.5-VL modeling files.
- [x] Remove Qwen2.5-VL (without padding) modeling files.
- [x] Remove related UT.
- [x] Make `set_forward_context` pluggable when getting MM embedding.
Find more details at vllm-project/vllm#29388.
- [x] Simplify padding logic for FA.
- [x] Add patch for vllm-project/vllm#28798.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

- [x] Functional test (eager mode)
- [x] Functional test (graph mode)
- [x] Benchmark

- vLLM version: v0.11.2

---------

Signed-off-by: shen-shanshan <467638484@qq.com>
Signed-off-by: Che Ruan <cr623@ic.ac.uk>
Mercykid-bash pushed a commit to Mercykid-bash/vllm-ascend that referenced this pull request Dec 4, 2025
@shen-shanshan @mercykid
[MM][Model][Perf] Remove Qwen2.5-VL modeling files and add patch for …
d5f3f59 
…VisionAttention (vllm-project#4349)

### What this PR does / why we need it?

- [x] Patch `Qwen2_5_VisionAttention` with
`AscendQwen2_5_VisionAttention`.
- [x] Replace `AscendQwen2_5_VisionTransformer` with
`Qwen2_5_VisionTransformer` in vllm.
- [x] Move padding logic (q/k/v and cos/sin) before FA to `forward()` of
`Qwen2_5_VisionAttention`.
- [x] Covert `cu_seqlens` in `Qwen2_5_VisionAttention` from cumulative
form to intervals and move it to cpu (compatible for npu FA).
- [x] Remove Qwen2.5-VL modeling files.
- [x] Remove Qwen2.5-VL (without padding) modeling files.
- [x] Remove related UT.
- [x] Make `set_forward_context` pluggable when getting MM embedding.
Find more details at vllm-project/vllm#29388.
- [x] Simplify padding logic for FA.
- [x] Add patch for vllm-project/vllm#28798.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

- [x] Functional test (eager mode)
- [x] Functional test (graph mode)
- [x] Benchmark

- vLLM version: v0.11.2

---------

Signed-off-by: shen-shanshan <467638484@qq.com>
Signed-off-by: Che Ruan <cr623@ic.ac.uk>
Meihan-chen pushed a commit to Meihan-chen/vllm-ascend that referenced this pull request Dec 5, 2025
@shen-shanshan @Meihan-chen
[MM][Model][Perf] Remove Qwen2.5-VL modeling files and add patch for …
3aa4e0f 
…VisionAttention (vllm-project#4349)

### What this PR does / why we need it?

- [x] Patch `Qwen2_5_VisionAttention` with
`AscendQwen2_5_VisionAttention`.
- [x] Replace `AscendQwen2_5_VisionTransformer` with
`Qwen2_5_VisionTransformer` in vllm.
- [x] Move padding logic (q/k/v and cos/sin) before FA to `forward()` of
`Qwen2_5_VisionAttention`.
- [x] Covert `cu_seqlens` in `Qwen2_5_VisionAttention` from cumulative
form to intervals and move it to cpu (compatible for npu FA).
- [x] Remove Qwen2.5-VL modeling files.
- [x] Remove Qwen2.5-VL (without padding) modeling files.
- [x] Remove related UT.
- [x] Make `set_forward_context` pluggable when getting MM embedding.
Find more details at vllm-project/vllm#29388.
- [x] Simplify padding logic for FA.
- [x] Add patch for vllm-project/vllm#28798.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

- [x] Functional test (eager mode)
- [x] Functional test (graph mode)
- [x] Benchmark


- vLLM version: v0.11.2

---------

Signed-off-by: shen-shanshan <467638484@qq.com>
Clorist33 pushed a commit to Clorist33/vllm-ascend that referenced this pull request Dec 9, 2025
@shen-shanshan
[MM][Model][Perf] Remove Qwen2.5-VL modeling files and add patch for …
6e97f8b 
…VisionAttention (vllm-project#4349)

### What this PR does / why we need it?

- [x] Patch `Qwen2_5_VisionAttention` with
`AscendQwen2_5_VisionAttention`.
- [x] Replace `AscendQwen2_5_VisionTransformer` with
`Qwen2_5_VisionTransformer` in vllm.
- [x] Move padding logic (q/k/v and cos/sin) before FA to `forward()` of
`Qwen2_5_VisionAttention`.
- [x] Covert `cu_seqlens` in `Qwen2_5_VisionAttention` from cumulative
form to intervals and move it to cpu (compatible for npu FA).
- [x] Remove Qwen2.5-VL modeling files.
- [x] Remove Qwen2.5-VL (without padding) modeling files.
- [x] Remove related UT.
- [x] Make `set_forward_context` pluggable when getting MM embedding.
Find more details at vllm-project/vllm#29388.
- [x] Simplify padding logic for FA.
- [x] Add patch for vllm-project/vllm#28798.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

- [x] Functional test (eager mode)
- [x] Functional test (graph mode)
- [x] Benchmark

- vLLM version: v0.11.2

---------

Signed-off-by: shen-shanshan <467638484@qq.com>
Signed-off-by: tanqingshan (A) <50050625@china.huawei.com>
Clorist33 pushed a commit to Clorist33/vllm-ascend that referenced this pull request Dec 10, 2025
@shen-shanshan
[MM][Model][Perf] Remove Qwen2.5-VL modeling files and add patch for …
15575b1 
…VisionAttention (vllm-project#4349)

### What this PR does / why we need it?

- [x] Patch `Qwen2_5_VisionAttention` with
`AscendQwen2_5_VisionAttention`.
- [x] Replace `AscendQwen2_5_VisionTransformer` with
`Qwen2_5_VisionTransformer` in vllm.
- [x] Move padding logic (q/k/v and cos/sin) before FA to `forward()` of
`Qwen2_5_VisionAttention`.
- [x] Covert `cu_seqlens` in `Qwen2_5_VisionAttention` from cumulative
form to intervals and move it to cpu (compatible for npu FA).
- [x] Remove Qwen2.5-VL modeling files.
- [x] Remove Qwen2.5-VL (without padding) modeling files.
- [x] Remove related UT.
- [x] Make `set_forward_context` pluggable when getting MM embedding.
Find more details at vllm-project/vllm#29388.
- [x] Simplify padding logic for FA.
- [x] Add patch for vllm-project/vllm#28798.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

- [x] Functional test (eager mode)
- [x] Functional test (graph mode)
- [x] Benchmark


- vLLM version: v0.11.2

---------

Signed-off-by: shen-shanshan <467638484@qq.com>
Mercykid-bash pushed a commit to Mercykid-bash/vllm-ascend that referenced this pull request Dec 10, 2025
@shen-shanshan @mercykid
[MM][Model][Perf] Remove Qwen2.5-VL modeling files and add patch for …
89b97c0 
…VisionAttention (vllm-project#4349)

### What this PR does / why we need it?

- [x] Patch `Qwen2_5_VisionAttention` with
`AscendQwen2_5_VisionAttention`.
- [x] Replace `AscendQwen2_5_VisionTransformer` with
`Qwen2_5_VisionTransformer` in vllm.
- [x] Move padding logic (q/k/v and cos/sin) before FA to `forward()` of
`Qwen2_5_VisionAttention`.
- [x] Covert `cu_seqlens` in `Qwen2_5_VisionAttention` from cumulative
form to intervals and move it to cpu (compatible for npu FA).
- [x] Remove Qwen2.5-VL modeling files.
- [x] Remove Qwen2.5-VL (without padding) modeling files.
- [x] Remove related UT.
- [x] Make `set_forward_context` pluggable when getting MM embedding.
Find more details at vllm-project/vllm#29388.
- [x] Simplify padding logic for FA.
- [x] Add patch for vllm-project/vllm#28798.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

- [x] Functional test (eager mode)
- [x] Functional test (graph mode)
- [x] Benchmark


- vLLM version: v0.11.2

---------

Signed-off-by: shen-shanshan <467638484@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ProExpertProg ProExpertProg ProExpertProg left review comments

@DarkLight1337 DarkLight1337 DarkLight1337 left review comments

@sighingnow sighingnow Awaiting requested review from sighingnow sighingnow is a code owner

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

qwen Related to Qwen models

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shen-shanshan @ProExpertProg @DarkLight1337