🚨🚨 Fix beam score calculation issue for decoder-only models
Author: VsonicV
Pull request: yes

# What does this PR do?

This PR fixes issue #26624 . In the original implementation of beam search, the beam score for decoder-only models is normalized by the total length of both prompt and generated sequence. However, the length of prompt should not be included in the normalization step. This issue would cause an unexpected bias towards generating shorter sequences.

This is a simple quick fix by adding an optional parameter `decoder_prompy_len`, which stores the length of prompt in decoder, to `BeamSearchScorer.process()`, `BeamSearchScorer.finalize()` and `BeamHypotheses.add()`. Since the added new parameter is optional with a default value as 0, any existing calls of these functions without specifying `decoder_prompy_len` would still work in the same way as before, avoiding any unexpected incompatibility. The corner case in which the very first generated token happens to be eos_token (empty generation) is considered and handled.

Fixes #26624 

Note: There are three follow-up PRs that complement this fix:
1. #27808 further fixes some remaining issues in the Pytorch version.
2. #27814 fixes the Tensorflow version.
3. #27816 fixes the JAX version.

## 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#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.
- [ ] 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?

@gante 


Comments:
@gante This commit is only for fixing `beam_search`. If you think the fix is good to go, I can also apply the same fix to `beam_sample`,  `group_beam_search` and `constrained_beam_search`.

@gante I think the current tests regarding `beam_search` are using the results generated by previous "buggy" version, so the new `beam_search` cannot pass the test `test_eos_token_id_int_and_list_beam_search`, which uses the decoder-only GPT-2. We need to update the relevant tests as well.

@VsonicV the "setup and quality" CI can be fixed by running `make fixup` on your local `transformers` root folder and committing the changes!

@gante Thanks for the suggestion! I have fixed the code quality issue using `make fixup`, and updated the relevant test `test_eos_token_id_int_and_list_beam_search` with new `expectation` value. Both checks pass now. However, there is still one check failure caused by `test_run_image_classification_no_trainer` and `test_run_ner_no_trainer`, which should be irrelevant to these commits regarding beam search. Do you have any clue about how to fix it?

@VsonicV perfect! The failing CI is indeed unrelated (fix: https://github.com/huggingface/transformers/pull/27353), the tests should pass after it gets merged.

To keep the consistency of this fix throughout beam methods, I'd like to request you to:
1. Also apply this change to other beam methods in Pytorch :)
2. Add 🚨🚨 to the PR title, as this is a (correct but also) breaking change
3. (optional, only if you're comfortable with it, as the fix is slightly different) Apply this change to beam methods in TF and JAX

After 1. and 2. is done, I'll tag a core maintainer to greenlight the merge!

@gante Sure! I will work on 1 and 2 in the next 2 days. Will try to do 3 after that.

The PR causing the CI to fail was merged, and I was informed that current PRs will need to be rebased to pass CI 🤗 

@gante Item 1 and 2 are done! I have applied the fix to all beam related methods: `beam_sample`, `group_beam_search` and `constrained_beam_search`. I have rebased the PR and all relevant tests have passed.

Regarding the remaining check failures, the recent merge only fixes the check failure caused by `test_run_image_classification_no_trainer`, but not for `test_run_ner_no_trainer`. According to the error message `AssertionError: 0.5109489440917969 not less than 0.5`, the checking threshold for `self.assertLess(result["train_loss"], 0.5)` in `test_run_ner_no_trainer` needs to be adjusted as well. Moreover, one new check failure is caused by `test_cached_model_has_minimum_calls_to_head` and `test_cached_tokenizer_has_minimum_calls_to_head`, which are unrelated to the commits in this PR (we only see this after the most recent rebase).

@VsonicV yes, we are still having some CI failures (unrelated to this PR) 😭 

@gante Tried rebasing once more, all the previous check failures are gone, but got one new CI failure caused by `test_assisted_decoding_sample`, which should again be unrelated to this PR.

@ArthurZucker I have rebased this PR with all your recently added test skips, the CI failures caused by `test_assisted_decoding_sample` still persist for `blenderbot`, same failures also happened for `pegasus` and `umt5` in my previous tries, would you mind adding skips of `test_assisted_decoding_sample` for `blenderbot`, `pegasus` and `umt5` as well? Thank you!

Yeah I'll skip this test for everyone this is getting annoying! 😅  #27511 was merged

Tagging @amyeroberts for a final check

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

@gante @amyeroberts All your suggested changes have been added and committed. All the tests have passed now (finally!). Should be ready for merge.

@VsonicV Thank you for iterating with us and making `transformers` better 💛

And sorry for all the failing CI, you've caught an unfortunate series of failures 😬 

@gante No problem! Regarding the fix of TF and JAX version, I have looked at the relevant codes briefly, and I think I can fix them. I will try to submit another PR fixing both TF and JAX later this week.

> @gante No problem! Regarding the fix of TF and JAX version, I have looked at the relevant codes briefly, and I think I can fix them. I will try to submit another PR fixing both TF and JAX later this week.

@gante Sorry about the delay in the next steps. I had a severe flu last week and just recovered. Will start working on the remaining fixes.

@gante @amyeroberts All follow-up tasks have been completed, in three new PRs:
1. #27808 further fixes some remaining issues in the Pytorch version.
2. #27814 fixes the Tensorflow version.
3. #27816 fixes the JAX version.

All three PRs have passed the CI checks. Ready for your review @gante .

@gante Hi, I noticed that in the recent release notes of v4.36.0, only this PR is listed in "Beam score calculation for decoder-only models" section under "Breaking changes". Should we also add the 3 follow-up PRs ( #27808 #27814 #27816 ) under that section? It would be more clear for people to check all the changes relevant to this breaking change. Thanks.

@VsonicV updated the release notes for future reference 👍 Thank you for your suggestion