Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should the default length_penalty be 0? #2564

Closed
galatolofederico opened this issue Jan 23, 2024 · 1 comment
Closed

Should the default length_penalty be 0? #2564

galatolofederico opened this issue Jan 23, 2024 · 1 comment

Comments

@galatolofederico
Copy link
Contributor

galatolofederico commented Jan 23, 2024

I'm wondering if the default value of length_penalty in the beam search algorithm should be set to 0.
My understanding, based on discussions like this issue in the hf transformers repository, is that a length_penalty greater than 0 favors shorter sequences, while a penalty less than 0 favors longer ones and 0 means no length_penalty

In the vLLM project (which has code adapted from Hugging Face), particularly in the file vllm/sequence.py, the default value is indeed 0:

def get_beam_search_score(self,
                          length_penalty: float = 0.0,
                          seq_len: Optional[int] = None,
                          eos_token_id: Optional[int] = None) -> float:
    """Calculate the beam search score with length penalty.
    Adapted from: https://github.com/huggingface/transformers/blob/ccb92be23def445f2afdea94c31286f84b89eb5b/src/transformers/generation/beam_search.py#L938
    """

Should we consider setting the default length_penalty to 0?

@hmellor
Copy link
Collaborator

hmellor commented Feb 7, 2024

The default length_penalty in get_beam_search_score was changed to 1.0 last week in #2667. So that it matches the one in SamplingParams.

This is the default because it's the default in transformers and the argument for this being the default in transformers is in huggingface/transformers#19006 (comment).

@hmellor hmellor closed this as completed Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants