Skip to content

Conversation

@wangxiyuan
Copy link
Collaborator

This reverts commit c87a77e.

it breaks ops e2e test

@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 reverts a previous change that generalized the gatingtopk operator, which was found to break end-to-end tests. The revert restores the previous, more specific implementation across vllm_ascend/ops/moe/experts_selector.py and related tests. The changes appear to correctly revert the problematic commit. I have one suggestion on the restored code in experts_selector.py to improve its robustness and prevent a potential bug where logic blocks could unintentionally overwrite each other.

# y2_flag=False, # old api; should the third output be output
routed_scaling_factor=1,
eps=float(1e-20))
if not use_grouped_topk and custom_routing_function is None and scoring_func == "softmax":
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The if statement on this line could potentially conflict with the preceding if is_deepseek_v3_r1: block. If conditions for both blocks are met (e.g., for a deepseek_v3_r1 model with scoring_func="softmax"), the results from the first block will be overwritten. This could lead to incorrect expert selection. Using elif would make these conditions mutually exclusive and prevent this potential bug.

Suggested change
if not use_grouped_topk and custom_routing_function is None and scoring_func == "softmax":
elif not use_grouped_topk and custom_routing_function is None and scoring_func == "softmax":

@wangxiyuan
Copy link
Collaborator Author

ops test should be updated


    def is_vl_model(vllm_config: VllmConfig):
        """Checks if the model is a VL model by config"""
        global _IS_VL_MODEL
        if _IS_VL_MODEL is None:
>           model_configs = vllm_config.model_config.hf_config.to_dict()
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
E           AttributeError: 'NoneType' object has no attribute 'hf_config'

@wangxiyuan wangxiyuan merged commit a2e4c3f into vllm-project:v0.11.0-dev Nov 21, 2025
39 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant