Skip to content

Fix logits_all parameter being ignored #4390

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

Closed
wants to merge 1 commit into from

Conversation

crasm
Copy link
Contributor

@crasm crasm commented Dec 9, 2023

batch.logits is always true since the new batch API mallocs it. This made the other control flows dead code.

I am unsure what the last else statement was intended to do, and removed it because it has been dead code for a while anyway.

`batch.logits` is always true since the new batch API mallocs it. This
made the other control flows dead code.

I am unsure what the last else statement was intended to do, and removed
it because it has been dead code for a while anyway.
@cebtenzzre
Copy link
Collaborator

cebtenzzre commented Dec 11, 2023

batch.logits is always true since the new batch API mallocs it.

Not true. llama_eval and llama_batch_get_one still exist and set it to nullptr. logits_all should not be used with the new batch API - instead, set all of batch.logits to true.

@cebtenzzre cebtenzzre closed this Dec 11, 2023
@crasm crasm deleted the fix-compute-all-logits branch December 11, 2023 23:01
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

Successfully merging this pull request may close these issues.

2 participants