Skip to content

[ROCm] Fix DeepSeek R1/V3 incorrect output in eager mode.#27392

Closed
Duyi-Wang wants to merge 1 commit intovllm-project:mainfrom
Duyi-Wang:fix_eager_mode_issue
Closed

[ROCm] Fix DeepSeek R1/V3 incorrect output in eager mode.#27392
Duyi-Wang wants to merge 1 commit intovllm-project:mainfrom
Duyi-Wang:fix_eager_mode_issue

Conversation

@Duyi-Wang
Copy link
Contributor

@Duyi-Wang Duyi-Wang commented Oct 23, 2025

Purpose

DeepSeek R1/V3 models produce incorrect output when running in eager mode, while graph mode works correctly.

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.

@mergify mergify bot added deepseek Related to DeepSeek models rocm Related to AMD ROCm labels Oct 23, 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 an issue where DeepSeek models produce incorrect output on ROCm in eager mode. The fix involves changing how forward_hip calls forward_cuda for rotary embeddings, correctly using the returned values instead of assuming an in-place operation. This change appears correct based on the problem description. I've added one comment to improve code clarity by removing a now-misleading comment.

Comment on lines 169 to 170
# ops.rotary_embedding() is an in-place operation
# that updates the query and key tensors.
Copy link
Contributor

Choose a reason for hiding this comment

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

high

These comments state that an internal operation is in-place, which contradicts the FIXME on the next line and the logic of the fix (which treats forward_cuda as not in-place by using its return value). To avoid confusion for future readers and maintainers, it would be better to remove these now-misleading comments.

@Duyi-Wang Duyi-Wang force-pushed the fix_eager_mode_issue branch from 47bdbdf to 8e17ff2 Compare October 23, 2025 12:21
@divakar-amd
Copy link
Contributor

A similar fix recently got merged: #27373

@Duyi-Wang Duyi-Wang closed this Oct 24, 2025
@github-project-automation github-project-automation bot moved this from Backlog to Done in DeepSeek V3/R1 Oct 24, 2025
@Duyi-Wang Duyi-Wang deleted the fix_eager_mode_issue branch November 18, 2025 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deepseek Related to DeepSeek models rocm Related to AMD ROCm

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants