Phi-3
Author: gugarosa
Pull request: yes
Labels: name: single-model-run-slow

# What does this PR do?

<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->

<!-- Remove if not applicable -->

Integrates Phi-3 within `transformers`.

## Before submitting
- [X] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [X] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [X] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [X] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [X] Did you write any new necessary tests?


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- text models: @ArthurZucker and @younesbelkada
- vision models: @amyeroberts
- speech models: @sanchit-gandhi
- graph models: @clefourrier

Library:

- flax: @sanchit-gandhi
- generate: @gante
- pipelines: @Narsil
- tensorflow: @gante and @Rocketknight1
- tokenizers: @ArthurZucker
- trainer: @muellerzr and @pacman100

Integrations:

- deepspeed: HF Trainer/Accelerate: @pacman100
- ray/raytune: @richardliaw, @amogkam
- Big Model Inference: @SunMarc
- quantization (bitsandbytes, autogpt): @SunMarc and @younesbelkada

Documentation: @stevhliu and @MKhalusova

HF projects:

- accelerate: [different repo](https://github.com/huggingface/accelerate)
- datasets: [different repo](https://github.com/huggingface/datasets)
- diffusers: [different repo](https://github.com/huggingface/diffusers)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Maintained examples (not research project or legacy):

- Flax: @sanchit-gandhi
- PyTorch: See Models above and tag the person corresponding to the modality of the example.
- TensorFlow: @Rocketknight1

 -->


Comments:
I bolded some questions that I had from the previous review. Could you please check them @ArthurZucker?

Done:
- Standardize how `Phi3LongScaledRotaryEmbedding` is validated/initialized. Uses the same approach defined by `Llama`.
- `swiglu` from `flash-attention` is required for now because we will need to ablate some alternative options. Will add a patch PR later on.
- Uses `copies` for the unit tests in `test_modeling_phi3.py`.
- Use `# Ignore copy` on `Phi3ForCausalLM.forward`.
- Apply `.float()` on the `apply_rotary_pos_emb`.
- Integration tests are added.

Review:
- **`DbrxAttention` has an extra clipping on the QKV, which prevents the copy. Or is there a way to ignore certain lines?**
- **Use old `get_usable_length` - did not catch the reference, checked Llama and Mistral files, and they are using it as well.**
- **`Phi3Model` uses an extra dropout - no sure how I should proceed with the copy. Can we ignore lines?**


I think documentation tests are fine, as they are only happening because they ran when phi-3-4k-instruct didn't exist. Waiting on approval for pull request

Hey! As I have mentioned offline I think that converting to llama format will allow anyone to use your model without any release (for any other frameworks), and we can add the new scalings ( SuScaledRotaryEmbedding,  YarnScaledRotaryEmbedding) separately! 

This would unlock the entire community! 🤗 

EDIT: this seems impossible as other frameworks are adapting to this format instead. We'll support the fused weights let's make the weights standardized for the good of the entire community! 🚀 🔥 

As mentioned in Slack, other team already sent a PR with the previous `Phi3LongScaledRotaryEmbedding` structure 😭 

But that should not impact the weights no? 

No, let me see what I can do!

Updated the rotary embedding classes. Fingers crossed that all tests will pass!

> EDIT: this seems impossible as other frameworks are adapting to this format instead. We'll support the fused weights let's make the weights standardized for the good of the entire community! 🚀 🔥

So no chance we could get a Llama-converted version?

> Overall LGTM, let's pay attention to ROPE and make it as simple and close to what our users are used to!

I think RoPE should be clearer now and closer to what transformers proposes. What do you think?

For now, I would like to keep the short_factor and long_factor on the config.json because I am still waiting for some replies on how they were created.

Solving the conflicts: accept all changes from main, ignore current

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_30423). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Hopefully I was able to address everything! But please let me know if anything else is needed

Hi a CI is triggered https://github.com/huggingface/transformers/actions/runs/8818618312

~~But since the CI in this PR is not green yet, you might consider to rebase on main once #30456 is merged into `main`.~~

Sorry about the mistake in the workflow file.

Well

=========== 100 passed, 47 skipped, 26 warnings in 391.83s (0:06:31) ===========

💯  

I ❤️  see this ! Thank you for the great contribution 🚀 

The circle CI seems crazy to run my branch #30455. If it is not green, it's fine. The last commit `aeb6ae7ebefcd4360441c2df4f20c0bcd45ff6f5` is green anyway

https://app.circleci.com/pipelines/github/huggingface/transformers?branch=pull%2F30423

Merging! 👍🏻 

Thanks for everything folks!

Thanks for yet another great contribution to the ecosystem ! 

Hi @gugarosa 

(I believe) The commit on Hub repo https://huggingface.co/microsoft/Phi-3-mini-4k-instruct/commit/3c0c9df9c11252fb61789d7847fa7d03f2825596

failed some tests 

```
tests/models/phi3/test_modeling_phi3.py::Phi3IntegrationTest::test_phi3_mini_128k_instruct_generation FAILED [ 99%]
tests/models/phi3/test_modeling_phi3.py::Phi3IntegrationTest::test_phi3_mini_4k_instruct_generation FAILED [100%]
```

see this job https://github.com/huggingface/transformers/actions/runs/8842226344/job/24280809384

Could you take a look and open a PR for a fix please?

You can run things like

```bash
RUN_SLOW=1 python3 -m pytest -rs -v tests/models/phi3/test_modeling_phi3.py::Phi3IntegrationTest::test_phi3_mini_4k_instruct_generation
```

Same for the example in the file

docs/source/en/model_doc/phi3.md

And the doc example for the class `Phi3ForCausalLM` doesn't seem working (from the first day)

```
from transformers import AutoTokenizer, Phi3ForCausalLM

model = Phi3ForCausalLM.from_pretrained("microsoft/phi-3-mini-4k-instruct")
tokenizer = AutoTokenizer.from_pretrained("microsoft/phi-3-mini-4k-instruct")

prompt = "This is an example script ."
inputs = tokenizer(prompt, return_tensors="pt")

# Generate
generate_ids = model.generate(inputs.input_ids, max_new_tokens=30)
o = tokenizer.batch_decode(generate_ids, skip_special_tokens=True, clean_up_tokenization_spaces=False)[0]
print(o)
```
It gives 

```bash
This is an example script .
```
which is just the input text itself.

Would be great if you can check this too, thanks.

https://github.com/huggingface/transformers/pull/30423#issuecomment-2079289950

Hi @gugarosa It would very nice if you could take a look 🤗 .