Skip to content

Llama Kernel integration#37092

Merged
MekkCyber merged 21 commits into
mainfrom
add_kernels_llama
Apr 10, 2025
Merged

Llama Kernel integration#37092
MekkCyber merged 21 commits into
mainfrom
add_kernels_llama

Conversation

@MekkCyber

Copy link
Copy Markdown
Contributor

What does this PR do?

PoC of Integrating kernels into the llama model. The kernels used are for attn, mlp, and rmsnorm

@github-actions

Copy link
Copy Markdown
Contributor

Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. The CI will be paused while the PR is in draft mode. When it is ready for review, please click the Ready for review button (at the bottom of the PR page). This will assign reviewers and trigger CI.

@github-actions github-actions Bot marked this pull request as draft March 28, 2025 20:13

@ArthurZucker ArthurZucker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Love the updates for rms and mlp!
Attention needs a bit more tinkering !
🤗 happy to hear your opinion as well!

class LlamaAttention(nn.Module):
"""Multi-headed attention from 'Attention Is All You Need' paper"""

use_kernel: bool = False

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

to remove!
Using kernels for that layer should IMO still be done via the attn_implementation!
it should give a bit more freedom to use kernels for the rest, and use sdpa or flex or paged or whatever here!

Attn is something that can change very easily, which is why I thought ALL_ATTENTION_FUNCTIONS was a better entry point!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you mean we pass something like attn_implementation="use_kenrel" in from_pretrained and then add a decorator with the name of the layer to use in the attention class ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes something along those lines. Or you can just say attn_implementation="kernels-community/paged-attention" something like this. (with extra (kernel_config = "...")
I don't know the answer but the simpler the better!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I changed the implementation so that we need to pass attn_implementation="use_kernel" and a kernel_config following the structure :
kernel_config={ "repo_id": "medmekk/triton-flash-attn", "kernel_name": "attention" }

_CONFIG_FOR_DOC = "LlamaConfig"


@use_kernel_forward_from_hub("LlamaRMSNorm")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is super nice!

@MekkCyber MekkCyber requested a review from ArthurZucker March 31, 2025 12:05
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@ArthurZucker ArthurZucker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Missing some docs a but marvelous

Comment thread src/transformers/modeling_utils.py Outdated
' We recommend to just use `attn_implementation="flash_attention_2"` when loading the model.'
)

if config._attn_implementation == "use_kernel":

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would allso support for example passing a repo _id directly no? We can get the device ourselves, this way its easy to just use 1 arg for simplicity!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We can create:

            "cuda": LayerRepository(
                repo_id="medmekk/triton-llama-mlp",
                layer_name="TritonLlamaMLP",
            )

base on the cls name for example

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we can have a func wrapper in integration / hub kernels or something

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

And if repo is not supper we juste use eager + warning!

@MekkCyber MekkCyber force-pushed the add_kernels_llama branch from 5611313 to 61de8e5 Compare April 7, 2025 16:08
@MekkCyber MekkCyber marked this pull request as ready for review April 9, 2025 08:05
@ArthurZucker

Copy link
Copy Markdown
Collaborator

Flag to disable is coming1

@MekkCyber MekkCyber merged commit 0ea1151 into main Apr 10, 2025
@MekkCyber MekkCyber deleted the add_kernels_llama branch April 10, 2025 15:13
@efsotr

efsotr commented Apr 12, 2025

Copy link
Copy Markdown
Contributor

I believe the implementation should be compatible with torch.compile.
For most simple modules, such as RMSNorm and operations like silu_and_mul in MLP, torch.compile already provides sufficient optimization. Therefore, for these cases, we should avoid overriding the forward method when torch.compile is enabled.

Suggestions:
In the kernels-community kernels, consider leveraging torch.compile for optimization, for example by using PyTorch ops or Triton implementations.
For custom kernels that are neither PyTorch nor Triton based, compatibility with torch.compile should also be taken into account.
For details on supporting custom kernels, refer to the official documentation: https://pytorch.org/docs/stable/library.html

@MekkCyber

MekkCyber commented Apr 14, 2025

Copy link
Copy Markdown
Contributor Author

Yeah that's right, once we have a stable api, we can think about how to make it compatible with torch.compile, but for now using kernels can be deactivated, by uninstalling the kernels library or just setting the DISABLE_KERNEL_MAPPING to 1 for example.
I'm also doing some benchmarks to see how much we gain or lose from using kernels instead of torch.compile, that would lay a good basis to see what we can do to improve the performance

cyr0930 pushed a commit to cyr0930/transformers that referenced this pull request Apr 18, 2025
* initial commit

* style

* update

* change approach attention

* clean up

* fix import

* update

* update

* fix style

* change method

* attention

* add mlp back

* change name

* update name

* fix copies

* fix config

* fix
zucchini-nlp pushed a commit to zucchini-nlp/transformers that referenced this pull request May 14, 2025
* initial commit

* style

* update

* change approach attention

* clean up

* fix import

* update

* update

* fix style

* change method

* attention

* add mlp back

* change name

* update name

* fix copies

* fix config

* fix
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.

4 participants