Update config validation
Author: Manalelaidouni
Pull request: yes

# What does this PR do?

Fixes #33690

This PR adds extra validation for generation parameters to fail early when invalid values are passed and not let things fail silently as suggested in the issue.  


## 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?
- [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.
- [ ]  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?

@gante @zucchini-nlp

Comments:
The failing tests seem unrelated to my changes and all tests passed locally for me, is there something I'm missing here?

Hey @zucchini-nlp thanks! I’ve added extra validations for the remaining generation parameters.

I also validated `bad_words_ids` inside  `GenerationConfig.validate()` ,  to test it before running the forward pass, not at the end of it (when its `NoBadWordsLogitsProcessor` is executed).

For now, I’ve left the `_validate_arguments()` method in `NoBadWordsLogitsProcessor` intact, I was considering removing it since it’s now redundant, but I’d like get your green light before proceeding; If approved I would apply the same approach for `sequence_bias` and adjust their tests accordingly :)

Thanks @zucchini-nlp ! I believe all generation parameters are validated now

Thank you @zucchini-nlp for reviewing, when adding tests I also made updates to some existing ones:

- So an older PR changed the `sequence_bias` format because it didn’t serialize properly, but the docstring and code still reference the dictionary format to users, which is confusing and since  `_convert_list_arguments_into_dict()` in `SequenceBiasLogitsProcessor`  is used for internal backwards compatibility, the `_validate_arguments()` should raise a ` ValueError`  if the input is not in the list format, right?

- I also adjusted the `forced_bos_token_id` and `forced_eos_token_id` and added missing `get_generation_mode`, the rest of parameters I validated in the config seem be already tested.

Finally, I’m not entirely sure about how to address  `bad_words_ids `  in the failing CI/CD tests, any guidance here would be appreciated, thank you again!



cc @ArthurZucker  whenever you have a moment, I'd appreciate your review, I wanted to see those CI tests green 😅 but it seems unrelated. Thanks!

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

Thanks for reviewing this @ArthurZucker!

- dataclasses don't inherently validate data, it won’t raise an error if passed an invalid data type, so we would need to implement the validation logic manually anyway, relies on passing default model configs via kwargs which makes fully converting it to a dataclass impractical.

- Since you mentioned using a tool, a much simpler solution would to use `attrs` and it’s easily scalable, but it adds an extra dependency ([Gante](https://github.com/huggingface/transformers/issues/33690#issuecomment-2444860878) advised against using `Pydantic` I assume because it significantly increases runtime).

- If we don't want to add dependencies, we can refactor the logic to a mapping approach which is scalable, something like this:

<br>

```python

class _ConfigValidator:

    VALIDATORS = {
        "pos_int_arg": (lambda x: isinstance(x, int) and x >= 0,
            "`{arg}` must be a positive integer, but got {value}."),

        "probability_arg": (lambda x: isinstance(x, (int, float)) and 0.0 <= x <= 1.0,
            "`{arg}` must be a float between 0 and 1, but got {value}."),
          ... }

    CONFIG_TYPES = {
        "top_k": "pos_int_arg",
        "top_p": "probability_arg",
         ...}


class GenerationConfig:
    ....

    def validate(self):
        for arg, value in self.__dict__.items():
            if arg in _ConfigValidator.CONFIG_TYPES:
                key = _ConfigValidator.CONFIG_TYPES[arg]
                validator, error_message = _ConfigValidator.VALIDATORS[key]
                if value is not None and not validator(value):
                    raise ValueError(error_message.format(arg=arg, value=value))


```

<br>

`_ConfigValidator` would hold also more complex validation for consistency and keep `GenerationConfig.validate()` less inflated for any future generation arguments. Let me know what you think before I move forward :)

I really don't mind using pydantic, it's become quite important and we have so many configs that we need to validate! Do you mind drafting a PR for that? 🤗 

Hey @ArthurZucker, apologies for the delayed response, I just opened a PR for it!

@Manalelaidouni this PR is orthogonal to the pydantic PR, correct? Or should we merge one before the other?

@gante they're indeed orthogonal this was the original plan, so I'll close this PR :)