Skip to content

Conversation

jeejeelee
Copy link
Collaborator

@jeejeelee jeejeelee commented Sep 23, 2025

Purpose

Ensure all LoRA linear respect the base_layer's tp_size and tp_rank

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.

Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
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 refactors the handling of tensor parallelism tp_size and tp_rank in LoRA layers to ensure consistency with the base layer, which is a solid improvement for maintainability and correctness. The changes also include some good optimizations to avoid unnecessary collective operations when tp_size is 1. Additionally, this PR fixes a critical bug in lora_weights.py where input_dim and output_dim properties were returning incorrect values. The changes are well-implemented and look good.

@@ -48,11 +48,11 @@ def optimize(self) -> "LoRALayerWeights":

@property
def input_dim(self) -> int:
return self.lora_a.shape[0]
return self.lora_a.shape[1]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

BTW, fix the dimension mismatch

@jeejeelee jeejeelee added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 23, 2025
Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
Copy link
Member

@Isotr0py Isotr0py 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!

@Isotr0py Isotr0py enabled auto-merge (squash) September 23, 2025 17:10
@Isotr0py Isotr0py merged commit 5abb117 into vllm-project:main Sep 23, 2025
46 checks passed
@jeejeelee jeejeelee deleted the lora-tp-optimize branch September 24, 2025 00:06
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
#25487)

Signed-off-by: Jee Jee Li <pandaleefree@gmail.com>
Signed-off-by: yewentao256 <zhyanwentao@126.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants