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

rotary embedding api interface for llama #15

Merged
merged 10 commits into from
Nov 10, 2023
Merged

rotary embedding api interface for llama #15

merged 10 commits into from
Nov 10, 2023

Conversation

aurora327
Copy link
Contributor

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like not formatted.
And, suggest separating .h and .cpp file.

@Duyi-Wang
Copy link
Contributor

Please use Camel case to rename variables and functions.

#include <iostream>

namespace xft {
void xftRotaryEmbeddingKernel(DataType dt,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just name RotaryEmbeddingKernel and used as xft::RotaryEmbeddingKernel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To keep the api interface consistent, align with this PR,layernorm
: #31

Copy link
Contributor

Choose a reason for hiding this comment

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

In that PR, changqing just use invokeLayerNorm in namespace xft instead of xftinvokeLayerNorm. Did I miss something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we want to maintain the consistency of the API interface to facilitate the users , such as 'Datatype' paramter. RotaryEmbedding added 'Datatype' parameter, and 'DataType' declare in in the namespace ‘xft ’,Thus RotaryEmbedding is also under the ‘xft’ namespace. This allows users to maintain a uniform approach when using Layernorm and RotaryEmbedding API

@Duyi-Wang Duyi-Wang merged commit c24a358 into intel:main Nov 10, 2023
1 check passed
Duyi-Wang pushed a commit that referenced this pull request Jul 18, 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