Skip to content

Conversation

@MengqingCao
Copy link
Contributor

@MengqingCao MengqingCao commented Oct 17, 2025

Purpose

Remove useless func get_input_positions in MRotaryEmbedding. Since #24172, we droped get_input_position_tensors in vLLM, and there is no use of get_input_positions anymore. This pr removes the useless func

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.

Copy link
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 is a straightforward refactoring that removes the unused get_input_positions function from the MRotaryEmbedding class, along with its now-unnecessary import. Based on the provided context and the pull request description, this function was indeed dead code. The removal is a good cleanup that improves code maintainability. The change is correct and I have no further suggestions.

Copy link
Member

@DarkLight1337 DarkLight1337 left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup!

@DarkLight1337 DarkLight1337 added the multi-modality Related to multi-modality (#4194) label Oct 17, 2025
@vllm-bot vllm-bot merged commit e20eba7 into vllm-project:main Oct 17, 2025
7 of 8 checks passed
@MengqingCao MengqingCao deleted the mrope branch October 17, 2025 09:17
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
albertoperdomo2 pushed a commit to albertoperdomo2/vllm that referenced this pull request Oct 23, 2025
alhridoy pushed a commit to alhridoy/vllm that referenced this pull request Oct 24, 2025
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

multi-modality Related to multi-modality (#4194)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants