Skip to content

ppl : fix n_seq_max for perplexity #8277

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

Merged
merged 2 commits into from
Jul 3, 2024
Merged

ppl : fix n_seq_max for perplexity #8277

merged 2 commits into from
Jul 3, 2024

Conversation

slaren
Copy link
Member

@slaren slaren commented Jul 3, 2024

The value of n_parallel (= n_seq_max) was being overwritten for hellaswag, which can result in the wrong rope factors being used with Phi3.

@mofosyne mofosyne added the Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix label Jul 3, 2024
Copy link
Collaborator

@ngxson ngxson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Nitpicks, but I'd prefer to reverse the bool ppl condition to be more readable:

const bool not_ppl = params.hellaswag || params.winogrande || params.multiple_choice || params.kl_divergence;

if (not_ppl) {
    ...
} else {
    ...
}

@ggerganov ggerganov merged commit 5f2d4e6 into master Jul 3, 2024
54 checks passed
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Jul 7, 2024
* ppl : fix n_seq_max for perplexity

* use 1 seq for kl_divergence
Nexesenex pushed a commit to Nexesenex/croco.cpp that referenced this pull request Feb 25, 2025
* ppl : fix n_seq_max for perplexity

* use 1 seq for kl_divergence
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants