Skip to content

Conversation

@zzhx1
Copy link
Contributor

@zzhx1 zzhx1 commented Nov 13, 2025

What this PR does / why we need it?

In torchair_mla.py, the self.oproj function includes an additional parameter is_force_scatter, while the AscendRowParallelLinear function in linear.py does not add this parameter.

@github-actions
Copy link

👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:‌‌

  • A PR should do only one thing, smaller PRs enable faster reviews.
  • Every PR should include unit tests and end-to-end tests ‌to ensure it works and is not broken by other future PRs.
  • Write the commit message by fulfilling the PR description to help reviewer and future developers understand.

If CI fails, you can run linting and testing checks locally according Contributing and Testing.

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 aims to fix a parameter mismatch for the o_proj forward pass by adding the is_force_scatter parameter to AscendRowParallelLinear.forward. While the parameter is correctly added to the method signature, it is not passed down to the custom operator's apply method. This makes the fix incomplete and the new parameter will have no effect. I've left a comment with a suggested code change to properly pass the parameter. Please note that this will also require further changes in vllm_ascend/ops/linear_op.py to update the signatures of the apply and apply_impl methods.

is_force_scatter: bool = False,
) -> Union[torch.Tensor, tuple[torch.Tensor, Optional[Parameter]]]:
if self.custom_op is not None:
return self.custom_op.apply(input_)
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

The newly added is_force_scatter parameter and the existing is_prefill parameter are not being passed to self.custom_op.apply(). This makes the fix incomplete as the custom op will not receive these values. You should pass them to the apply method. This will also require updating the signature of apply and apply_impl methods in CustomLinearOp and its subclasses in vllm_ascend/ops/linear_op.py.

Suggested change
return self.custom_op.apply(input_)
return self.custom_op.apply(input_, is_prefill=is_prefill, is_force_scatter=is_force_scatter)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant