noamaanMulla-03/transformers-issues huggingface dataset:
Update RayTune doc link for Hyperparameter tuning
Author: JoshuaEPSamuel
Pull request: yes

Link to RayTune search space API docs was outdated - have provided correct new link for docs.

# What does this PR do?

Updates broken link to RayTune search space API docs for the Transformers Hyperparameter tuning function. 

<!-- Remove if not applicable -->

Fixes #24135


## 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#start-contributing-pull-requests),
      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?

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.

@richardliaw , @amogkam 

<!-- 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: @sgugger

Integrations:

- deepspeed: HF Trainer/Accelerate: @pacman100
- ray/raytune: @richardliaw, @amogkam

Documentation: @sgugger, @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: @sgugger
- TensorFlow: @Rocketknight1

 -->


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_24422). All of your documentation changes will be reflected on that endpoint.
Added support for AzureOpenAiAgent in tools
Author: waundme
Pull request: yes

# What does this PR do?
Implements a new class AzureOpenAiAgent derived from Agent in transformer agents.

Fixes #23324

- [ ] 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#start-contributing-pull-requests),
      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).
- [ ] Did you write any new necessary tests?

Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_23355). All of your documentation changes will be reflected on that endpoint.

Awesome that you are contributing.
In my opinion, rather than having a separate AzureOpenAiAgent adding the options and API calls in the OpenAiAgent would be preferrable, so it is only configuration of parameters to switch to Azure. Not my call, but my preference. 

cc @sgugger 

Ah actually I see two arguments are renamed. Maybe have this be a subclass of `OpenAiAgent` to avoid rewriting every method and just rewrites `_completion_generate` and `_chat_generate`?

DeploymentId is named arbitrarily and does not let you directly derive the model type from it unless you do additional requests to look it up.
The underlying Python OpenAI SDK has a way of differentiating between Azure OpenAI and OpenAI's own deployment. In my opinion it would make sense to align the API style and expose what the API exposes in a similar fashion. 

> The underlying Python OpenAI SDK has a way of differentiating between Azure OpenAI and OpenAI's own deployment. In my opinion it would make sense to align the API style and expose what the API exposes in a similar fashion.

If it's easily doable, then yes let's aim for that!

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.
GPT2 and -100 in input_ids
Author: spyroot
Pull request: no

### System Info

I post all detail maybe it is expected behavior. 

### 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, ...)
- [x] My own task or dataset (give details below)

### Reproduction

Hi Folks,

I remember it used to work, but I may be missing something.  I noticed if I set the value in input_ids to -100.  
i.e., standard ignore value.   GPT-2 crash is essentially used as an index value where it should ignore it. 
Is there anything changed in GPT-2 model recently in the code base?  I'm not 100% sure is a bug or not. 

This led to a crash.   

```
attention_mask = batch['attention_mask']
mask_inverted = ~attention_mask.bool()
input_ids = batch["input_ids"]
input_ids = input_ids.masked_fill(mask_inverted == 1, -100).contiguous()
```

Thank you.

### Expected behavior

Expected behavior 
value in the input_ids should [id_n, id_n+1 etc,  eos_id,  -100]

Comments:
You can put -100 in the `labels` so they are ignored by the loss, but in the inputs as it's not a valid index for the embedding matrix.

Also this kind of questions is better suite for the [forums](https://discuss.huggingface.co/) as we keep GitHub issues for bugs and feature requests only.

Thank you very much for confirming, strange ... I have an old code that did work before I think it was masking -100. anyway looks like im a bit off here. : ) thank you!
`Trainer.hyperparameter_search` doesn't document `wandb` or offer it as a default backend
Author: alexmojaki
Pull request: no

`Trainer.hyperparameter_search` seems to prioritise the `optuna/ray/sigopt` backends, while `wandb` almost seems like a second-class citizen in the code. Specifically, the docstring explicitly mentions the first three backends multiple times in different contexts but not `wandb`, and `default_hp_search_backend` won't return `wandb` even if it's available. Is this intentional or accidental?

Comments:
Those are all integrations maintained by the authors of those libraries, we do not maintain them ourselves. It might be a bug, but it's up to the wandb folks to fix it in this case :-) 

Even the glue code in `trainer.py` that ties the various backends together?

Would you accept a PR to refactor this stuff? For example this code:

```python
        if backend is None:
            backend = default_hp_search_backend()
            if backend is None:
                raise RuntimeError(
                    "At least one of optuna or ray should be installed. "
                    "To install optuna run `pip install optuna`. "
                    "To install ray run `pip install ray[tune]`. "
                    "To install sigopt run `pip install sigopt`."
                )
        backend = HPSearchBackend(backend)
        if backend == HPSearchBackend.OPTUNA and not is_optuna_available():
            raise RuntimeError("You picked the optuna backend, but it is not installed. Use `pip install optuna`.")
        if backend == HPSearchBackend.RAY and not is_ray_tune_available():
            raise RuntimeError(
                "You picked the Ray Tune backend, but it is not installed. Use `pip install 'ray[tune]'`."
            )
        if backend == HPSearchBackend.SIGOPT and not is_sigopt_available():
            raise RuntimeError("You picked the sigopt backend, but it is not installed. Use `pip install sigopt`.")
        if backend == HPSearchBackend.WANDB and not is_wandb_available():
            raise RuntimeError("You picked the wandb backend, but it is not installed. Use `pip install wandb`.")
```

contains a lot of repetition that I'd be happy to clean up, and it's easy to see how the wandb integration author missed a place to add a reference to wandb.

The first bit with the runtime error is fine (though missing wandb). For the rest, it should be done in each integration which normally error very fast if the corresponding lib is not installed.
Refactor hyperparameter search backends
Author: alexmojaki
Pull request: yes

Fixes https://github.com/huggingface/transformers/issues/24379

The goal here is to clearly group the essential info/functionality about each backend together to make reading/changing things easier. For example if another backend integration is added it should be less likely for something to be forgotten as apparently happened with wandb.

@sgugger sorry I didn't get a full confirmation to go ahead with this, it just seemed easier to show what I meant with code rather than continue explaining in the issue. There's many other ways this could be done and I can change the approach but I hope that the general direction at least is clear from this PR.

I also think this would help move towards improving the user facing API since as mentioned in https://github.com/huggingface/transformers/issues/24278#issuecomment-1599189018 (cc @hugocool) the kwargs have no type hints and are not very easy to use. So maybe instead of:

```python
          best_run = trainer.hyperparameter_search(
                direction="maximize",
                backend="ray",
                # this is just **kwargs, not so clear what's possible...
                storage_path="...",
                callbacks=...,
            )  
```

one could write:

```python
          best_run = trainer.hyperparameter_search(
                direction="maximize",
                backend=RayTuneBackend(
                    # now more assistance is possible
                    storage_path="...",
                    callbacks=...,
                ),
            )  
```


Comments:
> Using abstract classes like this is not really the way the Transformers library is designed

That's fine, I was very unsure which approach to take. `abc` offers additional safety and IDE assistance as the standard way to ensure that all abstract methods are implemented, but it's probably overkill here and I also didn't like how heavy it was. I've pushed a much simpler strategy.

> I recommended to just complete the error message to include wandb.

The point of this is that it's difficult to see all the missing bits. The current code isn't just missing wandb in the error message, it's also missing from `default_hp_space` (fixed in this PR) and the docstring (not enforced in this PR, although it could be, I just didn't want to jump there just yet).

> I also don't see how you would have benefits for IDE as you show in the PR description.

Sorry for the confusion, that's not part of this PR to keep the scope focused, but if this is merged I can follow it up with another which adds constructors to each backend class which accept the precise kwargs that the backend `run` supports.

Opened https://github.com/huggingface/huggingface_hub/issues/1526 in regards to the unrelated test failure.

Thanks a lot for your contribution!

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_24384). All of your documentation changes will be reflected on that endpoint.
Save `site-packages` as cache in CircleCI job
Author: ydshieh
Pull request: yes

# What does this PR do?

Currently, we save `~/.cache/pip` as cache. Take `check_repository_consistency` job as example:
- it installs `[all, quality]`
- loading cache takes ~ 45 seconds
- `pip install` takes ~ 3-4 minutes

If we save `.pyenv/versions` as cache too:
- loading this new extra cache takes ~ 2 min
- `pip install` takes  20 ~ 30 seconds

We gain 30 ~ 90 seconds (depending on CircleCI's states). Not a big absolute improvement. But for this job which total runtime is ~ `5m30s`, we can say > 20% reduction. As `check_repository_consistency` and `check_code_quality` will be run for each PR's each push, probably it's nice to have such reduction.

WDYT?





Comments:
_The documentation is not available anymore as the PR was closed or merged._

>If we save .pyenv/versions as cache too:

>loading this new extra cache takes ~ 2 min
pip install takes 20 ~ 30 seconds


@ydshieh Wait - are the timings of these in the right order? 

> Do we still get updates if there is a release of one of the libraries?

@sgugger Yes if we use -U flag. I can do that if you are ok

> > If we save .pyenv/versions as cache too:
> 
> > loading this new extra cache takes ~ 2 min
> > pip install takes 20 ~ 30 seconds
> 
> @ydshieh Wait - are the timings of these in the right order?

Hmm, yes. But could you explain why you have doubts so I can reply in more details?

> @sgugger Yes if we use -U flag. I can do that if you are ok
YEs, please!

> Hmm, yes. But could you explain why you have doubts so I can reply in more details?

@ydshieh I just realised my mistake 🙃  I thought it was saying that it takes 2 mins to load with the cache and 30-40s to install by pip. Whereas it's (45 secs + 3-4 mins ) -> (2 mins + 20-30s). My bad! 

> > Hmm, yes. But could you explain why you have doubts so I can reply in more details?
> 
> @ydshieh I just realised my mistake 🙃 I thought it was saying that it takes 2 mins to load with the cache and 30-40s to install by pip. Whereas it's (45 secs + 3-4 mins ) -> (2 mins + 20-30s). My bad!


The new one should be (45 secs + 2 mins + 20-30s): The first part of cache (in `.cache/pip`) is not changed.
But we still have a little gain overall.



Although already been approved - FYI: I just added -U everywhere
[llama] Fix comments in weights converter
Author: weimingzha0
Pull request: yes

Explain the reason to clone tensor

The original comment doesn't explain much about why we need to do clone.


## 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.

@ArthurZucker @younesbelkada @sgugger 




Comments:
[AutoModel] Add AutoModelForTextEncoding
Author: sanchit-gandhi
Pull request: yes

# What does this PR do?

Adds AutoModel for text encoding (used in the circumstance when you want to extract the text encoder from an encoder-decoder architecture).

This facilitates loading a t5 encoder from t5 enc-dec model weights (as is done in Music Gen in #24109)

Comments:
_The documentation is not available anymore as the PR was closed or merged._
[`Trainer`] Fix `.to` call on 4bit models
Author: younesbelkada
Pull request: yes

# What does this PR do?

Currently the Trainer fails when calling initializing it on some scenarios using 4bit models
In fact, the device placement is correctly skipped for 8bit models but needs to be skipped as well for 4bit models as the `to` operation is not supported for 4bit models as well. 
This PR adds a patch for that case 

cc @amyeroberts @lewtun 


Comments:
_The documentation is not available anymore as the PR was closed or merged._

@amyeroberts thanks! 
Absolutely yes, for reference, here is how we set that attribute: https://github.com/huggingface/transformers/blob/ea91c2adca842da3d2f87e094504fa7d66a7008a/src/transformers/modeling_utils.py#L2922 

I can confirm that this fix resolves the error I was hitting with 4-bit models:

```
Traceback (most recent call last):
  File "/fsx/lewis/git/h4/scripts/evaluation/run_rm_eval.py", line 275, in <module>
    main()
  File "/fsx/lewis/git/h4/scripts/evaluation/run_rm_eval.py", line 164, in main
    trainer = RewardTrainer(
  File "/fsx/lewis/git/h4/src/h4/training/trainer.py", line 26, in __init__
    super().__init__(*args, **kwargs)
  File "/fsx/lewis/miniconda/envs/h4/lib/python3.10/site-packages/transformers/trainer.py", line 506, in __init__
    self._move_model_to_device(model, args.device)
  File "/fsx/lewis/miniconda/envs/h4/lib/python3.10/site-packages/transformers/trainer.py", line 747, in _move_model_to_device
    model = model.to(device)
  File "/fsx/lewis/miniconda/envs/h4/lib/python3.10/site-packages/transformers/modeling_utils.py", line 1889, in to
    raise ValueError(
ValueError: `.to` is not supported for `4-bit` or `8-bit` models. Please use the model as it is, since the model has already been set to the correct devices and casted to the correct `dtype`.
```

Thanks for the fast fix @younesbelkada !!
Fix some `TFWhisperModelIntegrationTests`
Author: ydshieh
Pull request: yes

# What does this PR do?

(Probably since the introduction of `GenerationConfig`), some TF Whisper Integration tests fail with error 

```bash
ValueError: The following `model_kwargs` are not used by the model: ['language', 'task'] (note: typos in the generate arguments will also show up in this list)
```

From my understanding, we should pass some arguments via `generation_kwargs`.

Note that `WhisperForConditionalGeneartion` has its custom `generate` but `TFWhisperForConditionalGeneartion` doesn't.

⚠️ When I try to apply the same changes to PyTorch Whisper test methods, it fails as the output is different.

We are somehow in a trouble of the inconsistency between PT/TF.

(Not sure if we should overwrite `generate` in `TFWhisperForConditionalGeneartion`.)

Comments:
_The documentation is not available anymore as the PR was closed or merged._

> LGTM- thanks for fixing!
> 
> Regarding the PyTorch tests failing with the same inputs - how do they fail?

It gives different outputs 

I am very sorry, **I was doing something stupid and made wrong claims**. The `generation_kwargs` is an argument to the `__init__` of `GenerationConfig` instead of the `generate` method. 

(Some of) The previous failing tests pass as the 2 problematic arguments to `generate` are not passed, but this is wrong logic.

**I will look in more depth.**

Well, I finally decided  to overwrite `generate` for `TFWhisperForConditionalGeneartion`.
Update `JukeboxConfig.from_pretrained`
Author: ydshieh
Pull request: yes

# What does this PR do?

The `from_pretrained` of `JukeboxConfig` needs an update (too) after #24306 to avoid the error `TypeError: __init__() got an unexpected keyword argument 'token'`.

Comments:
_The documentation is not available anymore as the PR was closed or merged._
Improved keras imports
Author: Rocketknight1
Pull request: yes

A sneaky bug was hiding in our Keras imports, where an import for `call_context` appeared to succeed on some TF versions, but actually got an older, unusable version of the function. This caused `build()` to behave improperly in some cases.

I went on a quest to fix this, and generally clean up our version-specific imports for TensorFlow to stop things like this from happening in future. I also bumped the minimum version for TF to 2.6 (2.6 should be 2 years old by the time of our next release), and eliminated the version cap in our dependency table because TF 2.13 should also be fully supported now.

This involved a partial rewrite of some code, where we checked for `KerasTensor` in a lot of places. However, this is an internal class for Keras and they keep moving it around, so trying to import it feels like a bad idea. Instead, I'm relying on `tf.is_tensor()`, which returns `True` for anything tensor-y, including symbolic tensors and Keras `Input` placeholders.

Fixes #24437 

Comments:
_The documentation is not available anymore as the PR was closed or merged._

> Keras and they keep moving it around

It's tensor-flow ... that's probably the reason.

I tested this with a spread of TF versions going back to 2.6. Even at 2.6, it was pretty hard to get an environment that worked with modern `transformers` - old TensorFlow keeps trying to use NumPy features that have been deprecated and deleted, but our modern libraries need a minimum version of NumPy to run at all, so there's actually only a very narrow window of NumPy versions that can even run both at once! I think going back to 2.5 or earlier would be very difficult, so I'm pretty comfortable with bumping our minimum version at this point.

In all versions I tested with this patch, our test suite runs well and the issue identified in #24437 is fixed, so this should be ready to  go after it's reviewed!
fix the grad_acc issue at epoch boundaries
Author: pacman100
Pull request: yes

# What does this PR do?

Should solve accumulating via epoch when using Accelerate (seen in https://github.com/huggingface/transformers/issues/23935#issuecomment-1588134562). Requires https://github.com/huggingface/accelerate/pull/1624

Fixes # (issue)

Comments:
_The documentation is not available anymore as the PR was closed or merged._

I believe this PR also fixes #24245

@amyeroberts we coordinate Accelerate releases to be a day or two before `transformers`, so there shouldn't be an issue there :) 

(Though @pacman100 we should do the version check like we've done before with these fixes 😬 )

Hello,

How do i install this? I expected that PR means some kind of transformers update, in this case there should be install link such as git+https://github.com/huggingface/transformers@de9255de27abfcae4a1f816b904915f0b1e23cd9




 


Hello @Oxi84, you can install this once it gets merged via `pip install git+https://github.com/huggingface/transformers` and `pip install git+https://github.com/huggingface/accelerate`

Just completed a training run with this PR and can confirm that the issue didn't occur. Thanks for the fix!