Skip to content

Conversation

gshtras
Copy link
Collaborator

@gshtras gshtras commented Sep 19, 2025

Follow up on #23412
If VLLM_ROCM_USE_AITER is enabled, and "-rms_norm" is explicitly set in the compilation config, an assertion is going to be thrown that an operation can't be both enabled and disabled.
Changing the enablement logic from the PR to respect explicit disablement.

Signed-off-by: Gregory Shtrasberg <Gregory.Shtrasberg@amd.com>
@mergify mergify bot added the rocm Related to AMD ROCm label Sep 19, 2025
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 addresses a bug where an assertion would be thrown if rms_norm was explicitly disabled while VLLM_ROCM_USE_AITER was enabled. The fix correctly checks if '-rms_norm' is present before attempting to enable it. I've added one suggestion to further improve the robustness of this logic by also considering the 'none' flag in custom_ops to prevent overriding a user's intent to disable all custom operations.

@gshtras gshtras added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 23, 2025
@ProExpertProg ProExpertProg merged commit 487745f into vllm-project:main Sep 24, 2025
53 checks passed
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
… disabled (#25275)

Signed-off-by: Gregory Shtrasberg <Gregory.Shtrasberg@amd.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 rocm Related to AMD ROCm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants