Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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.

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 3 commits into from
Feb 13, 2024
Merged

Conversation

lapp0
Copy link
Contributor

@lapp0 lapp0 commented Feb 12, 2024

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

@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!

@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.