Merged
Conversation
Member
|
Contributor
Author
|
I retried the ultravox and it is now giving the correct result on both metal and cpu Also just to note that, the original whisper impl from openai seem to use this type of gelu (with |
Contributor
Author
|
Here is the GGML output (conv + transformer + post layer norm): And python output: |
ggerganov
approved these changes
May 21, 2025
infil00p
pushed a commit
to baseweight/llama.cpp
that referenced
this pull request
May 22, 2025
* ggml : add ggml_gelu_na (not approximated) * fix naming order * rename na --> erf * apply review suggesions * revert naming order
This was referenced May 23, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I couldn't add it as a param because
ggml_geluis an unary op and I'm not quite sure how to add a new param to unary op (Please tell me if I should still add it as a param instead)Despite the name "not approximated", I'm actually using an approximation (more accurate than
tanh) in Metal impl because there is noerf()function build-in on Metal. The built-in GNU impl also seems to base on a complicated approx, so I think all systems are now using some short of approx, it's just more complicated thantanhso it gives a better result.So, maybe
ggml_gelu_nais not the best name.I haven't tested this with ultravox, will report back the result a bit later.