noamaanMulla-03/transformers-issues huggingface dataset:
Fix flash attention speed issue
Author: Cyrilvallez
Pull request: yes

# What does this PR do?

https://github.com/huggingface/transformers/pull/31961 inadvertently impacted the performance of the forward with FA2 quite negatively due to the very recurrent call to `is_flash_attn_greater_or_equal("2.4.1")`. This fixes the issue by adding a `lru_cache` to the function for fast lookup, allowing to retain the same performance as before the linked PR was merged.

## Simple benchmark

I ran the snippet on a RTX 4090 card

```python
from transformers import AutoModelForCausalLM, AutoTokenizer
import torch
import numpy as np

model_name = 'meta-llama/Meta-Llama-3-8B'
dtype = torch.bfloat16
model = AutoModelForCausalLM.from_pretrained(model_name, attn_implementation='flash_attention_2',
                                            torch_dtype=dtype, low_cpu_mem_usage=True).cuda()
tokenizer = AutoTokenizer.from_pretrained(model_name)
# Random token sequence
input = torch.randint(0, 1000, (1, 500), device='cuda')

times = []
for _ in range(10):
    start = torch.cuda.Event(enable_timing=True)
    end = torch.cuda.Event(enable_timing=True)
    start.record()
    # Generate 200 tokens
    out = model.generate(input, max_new_tokens=200, min_new_tokens=200, do_sample=False)
    end.record()
    torch.cuda.synchronize()
    times.append(start.elapsed_time(end) / 1000)  # time in seconds


print(np.mean(times))
```

This gives:

```python
# Before the fix
>>> 5.863476708984375 s

# After the fix
>>> 4.165076806640625 s
```


## Who can review?

@ArthurZucker  @gante

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

> LGTM, we can also just do it once, at the top of the file WDYT?

@ArthurZucker, usually I would agree if the function is only used in a single location, but in `modeling_gemma2.py` we also have an open call `softcap=self.config.attn_logit_softcapping if is_flash_attn_greater_or_equal("2.6.0") else None` in every FA2 forward call. Thus adding the `lru_cache` makes Gemma2 FA2 significantly faster as well (~2x on the previous snippet for Gemma2 9B). We could call the function a single time at the top of `modeling_gemma2.py` as well, but it may mean that we add large overhead to FA2 to other models in the future if we are not always careful about `is_flash_attn_greater_or_equal()`. Plus I am not aware of any downsides of `lru_cache` for this kind of helper functions (are there any?), so I think it works nicely in this case.
Can not detect bitsandbytes-windows
Author: ILG2021
Pull request: no
Labels: name: bug

### System Info

- `transformers` version: 4.39.3
- Platform: Windows-10-10.0.22631-SP0
- Python version: 3.10.0
- Huggingface_hub version: 0.24.2
- Safetensors version: 0.4.3
- Accelerate version: 0.33.0
- Accelerate config:    not found
- PyTorch version (GPU?): 2.4.0+cu118 (True)
- Tensorflow version (GPU?): not installed (NA)
- Flax version (CPU?/GPU?/TPU?): not installed (NA)
- Jax version: not installed
- JaxLib version: not installed
- Using GPU in script?: <fill in>
- Using distributed or parallel set-up in script?: <fill in>

### Who can help?

_No response_

### Information

- [ ] The official example scripts
- [ ] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

1. install bitsandbytes-windows
`python -m pip install https://github.com/jllllll/bitsandbytes-windows-webui/releases/download/wheels/bitsandbytes-0.41.2.post2-py3-none-win_amd64.whl`
2. load whisper model in 8 bit:
`model = WhisperForConditionalGeneration.from_pretrained(model_name_or_path, load_in_8bit=True, device_map="cuda:0")
`


### Expected behavior

Can correctly detect bitsandbytes and train model in 8bit on windows

Comments:
Hi @ILG2021,

Please install the official `bitsandbytes` distribution with:

`python -m pip install -U 'bitsandbytes>=0.43.3'`.

Hello, @matthewdouglas . Does it support windows? I mean not wsl.

@ILG2021 bitsandbytes has native Windows support [since v0.43.0](https://github.com/bitsandbytes-foundation/bitsandbytes/releases/tag/0.43.0).

Thank you very much, I get it.
split_between_processes not splitting dataset between processes?
Author: conceptofmind
Pull request: no

### System Info

python 3.10

### Who can help?

_No response_

### Information

- [ ] The official example scripts
- [X] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction
This does not work:
```python
dataset = load_dataset(
    "wikitext",
    "wikitext-2-v1",
    split="train",
)

print(len(dataset)) # 36718

# Assume 8 processes

with accelerator.split_between_processes(dataset) as data:
    print(len(data)) # 36718
    
   for out in pipe(
        KeyDataset(data, "text"), 
    ):
        continue
```
This works:
```python
dataset = load_dataset(
    "wikitext",
    "wikitext-2-v1",
    split="train",
)
print(len(dataset))

# Assume 8 processes

with accelerator.split_between_processes(dataset["text"]) as data:
    print(len(data))
```
### Expected behavior

I am unsure if this is intended behavior or not. A dataset consisting of the lists or input id tensors is split correctly. The dictionary provided from datasets will not be split.

Maybe this is more of a feature request?

Where the dataset could be split across all 8 processes to be used with the transformers feature extraction pipeline for inference.

Comments:
Hi @conceptofmind this is related to accelerate, you should open an issue there. cc @muellerz 

> Hi @conceptofmind this is related to accelerate, you should open an issue there. cc @muellerz

Hi @SunMarc,

I was unsure whether this should be listed under accelerate, datasets, or transformers as it is in relation to all of them. And I do not know whether each independent factor would impact the splitting of the data. 

Transformers - pipelines
Datasets - KeyDataset, load_dataset
Accelerate - split_between_processes
etc etc

I can reopen it under accelerate.

Thank you,

Enrico

Hi, @conceptofmind 

Did you find a fix for this issue?
Add SDPA implementation to more models
Author: avishaiElmakies
Pull request: no
Labels: name: Feature request

### Feature request

many of the newer models have an sdpa implementation. It could be good that some of the older models get up to speed.

### Motivation

this could be good to unify the API more.
and improve training\inference speed for some of the older models like OPT.

### Your contribution

Might be able to help with a few. this one seems bigger than the other feature request i opened, so will probably need more help

Comments:
Hey @avishaiElmakies, this seems like a duplicate of https://github.com/huggingface/transformers/issues/26350

We'd very much welcome some SDPA additions :hugs: 

@LysandreJik, it says flash attention. So I thought it was a different thing 😅. You can close this if you consider them the same issue. I will try to work on some models in my free time.

 

It's #28005 ;) 

Thanks

Thanks @vasqu!
Add missing quotes in modeling_llava_next_video.py
Author: juliendenize
Pull request: yes

# What does this PR do?

This PR add a missing quote in the documentation of LLavaNextVideo model.

## Before submitting
- [x] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [ ] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [ ] 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.
- [ ] 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).
- [ ] Did you write any new necessary tests?


## Who can review?

@amyeroberts
@stevhliu

Comments:
Pipeline: fix bad generation kwargs docs
Author: gante
Pull request: yes

# What does this PR do?

Fixes other occurrences of the bad pattern found in #32491 

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_33205). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Add warning for stop string edge case
Author: Rocketknight1
Pull request: yes

In #31435 @murray-z mentions that the stop string code can throw an error with an unusual stop string: "✿RESULT✿"

The underlying cause is that the ✿ token doesn't exist in the tokenizer vocabulary anywhere. The tokenizer in question has a special-case fallback for unknown characters where it builds them out of other tokens, but the tokens it uses to compose unknown characters have different meanings when considered alone. This means that the stop string code can't figure out how to build the stop string out of the tokenizer vocabulary, because it doesn't have access to the internal special-casing logic.

I can't really see a general solution here without completely rewriting the stop string code to run `tokenizer.decode()` at each generation step, which would break the ability to compile generation or export it to non-Python environments. Since it's a fairly rare edge case, though, I'm just going to add a proper error message instead!

cc @LysandreJik 

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_33169). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Local repos not correctly registering for pipelines
Author: Rocketknight1
Pull request: no
Labels: name: Core: Pipeline

If I load a custom code causal LM from the Hub, it is usable in a text generation pipeline, for example:

```python
from transformers import pipeline
pipe = pipeline("text-generation", "internlm/internlm2_5-7b-chat")
```

However, if I clone the same repo to my local machine and load it, this throws an error:

```python
from transformers import pipeline
pipe = pipeline("text-generation", "./internlm2_5-7b-chat")
```

outputs

```
The model 'InternLM2ForCausalLM' is not supported for text-generation. Supported models are ['BartForCausalLM', 'BertLMHeadModel', 'BertGenerationDecoder', 'BigBirdForCausalLM', 'BigBirdPegasusForCausalLM', 'BioGptForCausalLM', 'BlenderbotForCausalLM', 'BlenderbotSmallForCausalLM', 'BloomForCausalLM', 'CamembertForCausalLM', 'LlamaForCausalLM', 'CodeGenForCausalLM', 'CohereForCausalLM', 'CpmAntForCausalLM', 'CTRLLMHeadModel', 'Data2VecTextForCausalLM', 'DbrxForCausalLM', 'ElectraForCausalLM', 'ErnieForCausalLM', 'FalconForCausalLM', 'FalconMambaForCausalLM', 'FuyuForCausalLM', 'GemmaForCausalLM', 'Gemma2ForCausalLM', 'GitForCausalLM', 'GPT2LMHeadModel', 'GPT2LMHeadModel', 'GPTBigCodeForCausalLM', 'GPTNeoForCausalLM', 'GPTNeoXForCausalLM', 'GPTNeoXJapaneseForCausalLM', 'GPTJForCausalLM', 'JambaForCausalLM', 'JetMoeForCausalLM', 'LlamaForCausalLM', 'MambaForCausalLM', 'Mamba2ForCausalLM', 'MarianForCausalLM', 'MBartForCausalLM', 'MegaForCausalLM', 'MegatronBertForCausalLM', 'MistralForCausalLM', 'MixtralForCausalLM', 'MptForCausalLM', 'MusicgenForCausalLM', 'MusicgenMelodyForCausalLM', 'MvpForCausalLM', 'NemotronForCausalLM', 'OlmoForCausalLM', 'OpenLlamaForCausalLM', 'OpenAIGPTLMHeadModel', 'OPTForCausalLM', 'PegasusForCausalLM', 'PersimmonForCausalLM', 'PhiForCausalLM', 'Phi3ForCausalLM', 'PLBartForCausalLM', 'ProphetNetForCausalLM', 'QDQBertLMHeadModel', 'Qwen2ForCausalLM', 'Qwen2MoeForCausalLM', 'RecurrentGemmaForCausalLM', 'ReformerModelWithLMHead', 'RemBertForCausalLM', 'RobertaForCausalLM', 'RobertaPreLayerNormForCausalLM', 'RoCBertForCausalLM', 'RoFormerForCausalLM', 'RwkvForCausalLM', 'Speech2Text2ForCausalLM', 'StableLmForCausalLM', 'Starcoder2ForCausalLM', 'TransfoXLLMHeadModel', 'TrOCRForCausalLM', 'WhisperForCausalLM', 'XGLMForCausalLM', 'XLMWithLMHeadModel', 'XLMProphetNetForCausalLM', 'XLMRobertaForCausalLM', 'XLMRobertaXLForCausalLM', 'XLNetLMHeadModel', 'XmodForCausalLM'].
```

This issue is masked in many cases because a lot of custom code models use architecture strings in their configs that match existing models. For example, Phi-3.5 is a custom code model, but it uses a `phi3` architecture tag in its config, and this means it hits a match in the model mapping list even when using a local repo.

Filing an issue because it's not directly related to what I'm working on right now, but I'll need to investigate this at some point!

Comments:
Fix local repos with remote code not registering for pipelines
Author: Rocketknight1
Pull request: yes

Draft PR, don't merge! Experimenting with fixes for the issue with local repos and remote code not registering correctly for pipelines.

TODO:
 - [x] Add a test
 - [x] Maybe run some slow autoclass/pipeline tests to make sure I'm not breaking anything
 - [x] Try removing `model_class.register_for_auto_class(cls.__name__)` entirely?

Fixes #32923

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

This is now ready for review! The explanation for it is a bit confusing, though. The [original issue](https://github.com/huggingface/transformers/issues/32923) is that custom code models did not correctly register with the pipeline model mappings when they were loaded from a local directory instead of from the Hub. The cause is this block of code that treats models differently when they are loaded from a local dir:

```python
if os.path.isdir(config._name_or_path):
    model_class.register_for_auto_class(cls.__name__)
else:
    cls.register(config.__class__, model_class, exist_ok=True)
```

When I investigated, this code block was [added](https://github.com/huggingface/transformers/pull/24785) by @sgugger to fix [another issue](https://github.com/huggingface/transformers/issues/24737), where custom code models loaded from local directories did not correctly save their modeling files when saved with `save_pretrained()`.

In testing, I tried reverting this change, and it appears that custom code models now correctly save those files even after the revert.

Therefore, this PR reverts the change, and adds two new tests, one test to ensure that loading a local custom code model in a pipeline works now (this test fails without this PR), and one test to ensure that custom code models loaded from a local dir still correctly save their modeling files (this test passes on `main`, but I'm adding it as an anti-regression precaution).

cc @LysandreJik 

@LysandreJik done!
Refactor CI: more explicit
Author: ArthurZucker
Pull request: yes

# Rework the way we pass tests lists to the circle CI jobs
1. We no longer pass entire folders, and each job has a list of files, filtered from the output of the `test_fetcher`
2. All job's files are listed on the `fetch_test`'s artifacts
3. Added a `generation` job, and the corresponding markers for the generate jobs. 
5. `torch` jobs exclude generate markers
6. Tests are split based on previously saved timings info for faster CIs

To avoid too long config.yaml on the continuation orb, we pass the links to the artifacts as `parameters`. 
We used:
- https://circleci.com/docs/api/v2/index.html#operation/getJobArtifacts : to get the job artifacts

We could not pass a nested dict, which is why we create the `test_file_...` parameters for every single job

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

Maybe onnx sould be kept if the fetcher does not detect that if modeling files are touched they should be run as well

The 2 new files prepared by test fetcher should be used in `.circleci/create_circleci_config.py` I belive.

yes! not over yet sorry. I realized afterwards 

I will play with it and review!

@ArthurZucker  Let me know if you have any comment based on my above review. Once we achieve some agreement, I will finalize the PR.

Sorry did not have time to finish

PR IS DONE!
Create local Transformers Engine
Author: aymeric-roucher
Pull request: yes
Labels: name: Agents

# What does this PR do?

Renames HfEngine to HfApiEngine and creates a new TransformersEngine to support local llm pipelines.
Solves #32245.

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_33218). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Agent LLM Engine Support Local Inference
Author: swtb3
Pull request: no
Labels: name: Feature request

### Feature request

The provided HfEngine class uses the inference API under the hood, this makes using agents simple. However, it would be good to support local inference in a similarly simple way.

If this is already supported through existing local inference pipelines (text-generation pipeline) then documentation should disambiguate the standard approach to local inference with agents. 

### Motivation

It was unclear when following the tutorial how to approach local inference with agents as it uses inference API by default.

### Your contribution

Happy to make these changes, would need some support on the approach.

### Update

I have had some success replacing the client with a pipeline, though this is a bit messy and took a fair bit of trial and error.

I'm so far unable to get the working local agent to work with the gradio chat interface.

Comments:
cc @aymeric-roucher 

I also have this request

@gxcuit you can view my implementation here:

https://github.com/swtb3/math_agent_demo

It's not the cleanest, but it does work for local inference with a hf pipeline. I also tried to add support for ollama. However I found that these ollama models were unable to properly leverage the agent react framework. And often they failed to perform their role.

The HF pipeline worked really well though. 

Thank you for this feature request @swtb3 ! In the PR #33218 that I've merged above, your point should be addressed: now you can just initialize a `TransformersEngine` with your custom transformers `pipeline`! cc @gxcuit
Fix red amin
Author: ArthurZucker
Pull request: yes

# What does this PR do?
Since now we always have the parameters. If they are not passed, then the job should not even be triggered! 

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_33220). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Use Self type for PreTrainedModel.from_pretrained
Author: alanhdu
Pull request: yes

# What does this PR do?

This type is more precise and more accurately captures the type-signature. Without it, in our codebase we get a bunch of type-checking errors like:

```
from transformers import BertForTokenClassification

# Incompatible attribute type [8]: Variable `model` has type `BertForSequenceClassification` but is used as type `PreTrainedModel`
model: BertForTokenClassification = BertForTokenClassification.from_pretrained(...)
```


## Before submitting
- [ ] 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?
- [ ] 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.
- [ ] 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).
- [ ] Did you write any new necessary tests?


## Who can review?

@amyeroberts 


Comments:
@alanhdu The quality checks will need to pass before we can merge. I believe running `make fixup` and pushing the changes should resolve. 

hmmmm... I had a little niggling feeling `make fixup` might not resolve. Could you try manually addressing the failures?
Test fetcher: missing return on filtered tests; don't write empty files
Author: gante
Pull request: yes

# What does this PR do?

See title :)

This PR addresses two problems:
- missing return
- writing empty files

# missing return

The `filter_tests(tests, module=module)` call always returned `None`, because it is missing a `return`. This `None` would then cause the fetcher to fail: https://app.circleci.com/pipelines/github/huggingface/transformers/102546/workflows/5e358605-da6b-4c2f-886f-84c9e455a211/jobs/1365782

With this return, the failing job above no longer happens, and CI runs as expected :) (see [this PR](https://github.com/huggingface/transformers/pull/33219))

# writing empty files

The test fetcher can write empty files for a given job (e.g. `test_preparation/tests_torch_and_tf_test_list.txt`). This caused a downstream error because:
- CircleCI skips uploads of empty files (see [here](https://app.circleci.com/pipelines/github/huggingface/transformers/102551/workflows/263f91e9-aff9-41fe-ab51-d48440ac4cec/jobs/1365844), in `Uploading artifacts`)
- We trigger a job when there is a file (before upload)
- The job tried to download a file that didn't exist, causing a failure in the `curl` step (see [here](https://app.circleci.com/pipelines/github/huggingface/transformers/102551/workflows/aa0f93e6-30b6-412a-887c-322edc56e81a/jobs/1365855))

As such, this PR changes our test fetcher to skip writing a test file if the file is empty 🤗 No file -> no job -> no problem

Comments:
(see slack convo: https://huggingface.slack.com/archives/C014N4749J9/p1725033526299719)

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_33224). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.
Generate: throw warning when `return_dict_in_generate` is False but should be True
Author: gante
Pull request: yes

# What does this PR do?

See title. 

I've fallen victim to this one a few times, so it probably means users have fallen too :)

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