[LLaMA] Rotary positional embedding differs with official implementation
Author: lytning98
Pull request: no

`transformers` implement LLaMA model's Rotary Positional Embedding (RoPE) as follows:

https://github.com/huggingface/transformers/blob/e42587f596181396e1c4b63660abf0c736b10dae/src/transformers/models/llama/modeling_llama.py#L173-L188

This is **GPT-NeoX style** RoPE. But in Meta's official model implementation, the model adopts **GPT-J style** RoPE, which processes query and key vectors in an **interleaved way** instead of split into two half (as in `rotate_half` method).

Meta's official repo implements RoPE as ([full code link](https://github.com/facebookresearch/llama/blob/6c7fe276574e78057f917549435a2554000a876d/llama/model.py#L64-L74)):

```python
def apply_rotary_emb(
    xq: torch.Tensor,
    xk: torch.Tensor,
    freqs_cis: torch.Tensor,
) -> Tuple[torch.Tensor, torch.Tensor]:
    xq_ = torch.view_as_complex(xq.float().reshape(*xq.shape[:-1], -1, 2))
    xk_ = torch.view_as_complex(xk.float().reshape(*xk.shape[:-1], -1, 2))
    freqs_cis = reshape_for_broadcast(freqs_cis, xq_)
    xq_out = torch.view_as_real(xq_ * freqs_cis).flatten(3)
    xk_out = torch.view_as_real(xk_ * freqs_cis).flatten(3)
    return xq_out.type_as(xq), xk_out.type_as(xk)
```

I'm confused with this difference, since `transformers.LlamaModel` can directly load weights converted from the officially released checkpoint, won't this lead to inconsistency in inference results? Is this difference expected?

Comments:
same confusion

> same confusion

@santiweide Params of some layers are re-permuted while converting weights in the official scripts. Check

https://github.com/huggingface/transformers/blob/e42587f596181396e1c4b63660abf0c736b10dae/src/transformers/models/llama/convert_llama_weights_to_hf.py#L113-L115

ohhh thank you, we are converting  the Megatron weight to ft weight, and we would check the shape of weights then

Awesome, thanks for clarifying this! 


Awesome, thanks for clarifying this!



Thanks for the detailed illustration!!!

Thank you @lytning98, your answer saved my life.

May I ask the purpose behind this process? 
https://github.com/huggingface/transformers/blob/6cdbd73e01a9719bfaec07d91fd108e8d932bbbb/src/transformers/models/llama/modeling_llama.py#L177-L181
I mean, why not use the interleaved pair as in Meta's official llama?
@zphang @ArthurZucker .

Thanks in advance.


> Thank you @lytning98, your answer saved my life.
> 
> May I ask the purpose behind this process?
> 
> https://github.com/huggingface/transformers/blob/6cdbd73e01a9719bfaec07d91fd108e8d932bbbb/src/transformers/models/llama/modeling_llama.py#L177-L181
> 
> 
> I mean, why not use the interleaved pair as in Meta's official llama?
> @zphang @ArthurZucker .
> Thanks in advance.

https://discuss.huggingface.co/t/is-llama-rotary-embedding-implementation-correct/44509/2

Few reasons that are already mentioned:
- first and foremost, and I can't stress this enough, licence 
- second, eleuther's rope formulation (that we are using) is equivalent, maybe has one less operation that makes it more optimised 

@santiweide I am also converting my MegatronGPTModel to HF and seeing that the outputs from the models are totally different. How did you solve the issue?