Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add comments on RoPE initialization #1176

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Add comments on RoPE initialization #1176

merged 1 commit into from
Sep 26, 2023

Conversation

WoosukKwon
Copy link
Collaborator

@WoosukKwon WoosukKwon commented Sep 25, 2023

Added more comments that can be useful for understanding the differences from HF.

@casper-hansen
Copy link
Contributor

casper-hansen commented Sep 26, 2023

Does this explain the difference between HF and vLLM? I.e. if you enable the same CPU initialization with greedy sampling, we get same outputs?

@WoosukKwon
Copy link
Collaborator Author

@casper-hansen Not always. Because floating-point arithmetics is not associative, different kernel implementations might lead to different outputs. The difference is more significant when reduction operation is involved. Therefore, our custom CUDA kernels for attention and RMS normalization do not produce exactly the same outputs as the original HF implementation, and thus outputs of vLLM models can be different from the HF models. We've checked that the outputs usually match when using FP32 and greedy sampling, but there are some cases where the outputs do not match. However, please note that this does not hurt task accuracy, as vLLM's implementation is mathematically equivalent to HF's.

Copy link
Member

@zhuohan123 zhuohan123 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the fix!

@WoosukKwon WoosukKwon merged commit 03ffd0a into main Sep 26, 2023
2 checks passed
@WoosukKwon WoosukKwon deleted the fix-rope branch September 26, 2023 17:48
hongxiayang pushed a commit to hongxiayang/vllm that referenced this pull request Feb 13, 2024
sjchoi1 pushed a commit to casys-kaist-internal/vllm that referenced this pull request May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants