8000 fix sample_idx off-by-one error by lapp0 · Pull Request #1179 · abetlen/llama-cpp-python · GitHub
[go: up one dir, main page]
Skip to content

fix sample_idx off-by-one error #1179

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 13, 2024
Merged

fix sample_idx off-by-one error #1179

merged 3 commits into from
Feb 13, 2024

Conversation

lapp0
Copy link
Contributor
@lapp0 lapp0 commented Feb 12, 2024 •
edited
Loading

Fixes an off-by-one error when applying logits processor.

In this example, a draft model isn't used. The sample_idx is set to one less than the length of the input tokens, resulting in the last token being truncated.

(Pdb) idx
8
(Pdb) self._input_ids
array([    1,  1824,   349,   272,  1141,   302,  5459,  9955, 28804],
      dtype=int32)
(Pdb) len(self._input_ids)
9
(Pdb) self._input_ids[:idx]
array([   1, 1824,  349,  272, 1141,  302, 5459, 9955], dtype=int32)

@abetlen Would appreciate a review since you have a better understanding of the sample_idx argument. Especially considering this will impact logits = self._scores[idx, :]

Related: dottxt-ai/outlines#638

@lapp0 lapp0 mentioned this pull request Feb 12, 2024
Closed
@abetlen
Copy link
Owner
abetlen commented Feb 13, 2024

@lapp0 I'll look into this, is it specifically an issue with the logits_processor?

@lapp0
Copy link
Contributor Author
lapp0 commented Feb 13, 2024

@abetlen yes it is.

@lapp0 lapp0 force-pushed the fix-sample_idx-off-by-one branch from 447b511 to 493d6a7 Compare February 13, 2024 16:54
@abetlen
Copy link
Owner
abetlen commented Feb 13, 2024

@lapp0 thank you, looks good to me!

@abetlen abetlen merged commit d6be533 into abetlen:main Feb 13, 2024
@isamu-isozaki isamu-isozaki mentioned this pull request Feb 13, 2024
Closed
@lapp0
Copy link
Contributor Author
lapp0 commented Feb 13, 2024

Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lapp0 @abetlen
0