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.

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

Update logits array in-place #859

Open
brandonwillard opened this issue May 2, 2024 · 6 comments · Fixed by #1192 · May be fixed by lapp0/outlines#92
Open

Update logits array in-place #859

brandonwillard opened this issue May 2, 2024 · 6 comments · Fixed by #1192 · May be fixed by lapp0/outlines#92
Assignees
@lapp0 @rlouf
Labels
enhancement help wanted optimization Related to performance optimizations structured generation Linked to structured generation

Comments

@brandonwillard
Copy link
Member

brandonwillard commented May 2, 2024 •
edited
Loading

What behavior of the library made you think about the improvement?

The current structured generation code is creating a -inf copy of the logits array and setting the allowed token ID indices to the corresponding values in the original logits array. See here.

How would you like it to behave?

When possible, the original logits array should be updated in-place and completely avoid creating a new array. This change would likely require the set of disallowed token IDs instead of the allowed ones.
@brandonwillard brandonwillard added enhancement optimization Related to performance optimizations structured generation Linked to structured generation help wanted labels May 2, 2024
@brandonwillard
Copy link
Member Author

@rlouf @lapp0

@lapp0
Copy link
Contributor

lapp0 commented Sep 26, 2024

This is dead code which should be cleaned up after

#1010

@brandonwillard
Copy link
Member Author

This is dead code which should be cleaned up after

#1010

The relevant code now seems to be here; is that what will be updated in #1010?

@lapp0
Copy link
Contributor

lapp0 commented Sep 26, 2024

No, that is a problem, thanks for pointing it out. I'll look at it once my current task is complete.

This was referenced Oct 5, 2024
Open
Merged
@brandonwillard brandonwillard reopened this Oct 7, 2024
@brandonwillard
Copy link
Member Author

brandonwillard commented Oct 7, 2024 •
edited
Loading

#1192 does not appear to update the logits arrays in place, at least not without creating another array the size of the logits.

@brandonwillard
Copy link
Member Author

Looks like this might need to be moved after #1206.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lapp0 lapp0

@rlouf rlouf

Labels
enhancement help wanted optimization Related to performance optimizations structured generation Linked to structured generation
Projects
None yet
Milestone
No milestone
3 participants
@brandonwillard @lapp0 @rlouf