-
Notifications
You must be signed in to change notification settings - Fork 158
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
Add integer and enum types #19
base: main
Are you sure you want to change the base?
Conversation
jsonformer/logits_processors.py
Outdated
if ( | ||
len(decoded) > 1 | ||
and any(c.isdigit() for c in decoded) | ||
and decoded[-1] in [" ", "\n"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to check all chars to account for the case where the last sampled token was something like 1<space>h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be necessary since OutputIntegersTokens
only allows tokens consisting of digits with optional leading and trailing whitespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was just doing some more testing with this and am now running into some bugs when these features are used with llama-7b and dolly-v2-12b.
Everything seems to be working now. One of the issues was actually a problem with both integer generation and number generation so that bug is also fixed. The model wasn't actually being allowed to generate a comma, which would actually be the correct way to terminate a JSON number. The result was that the numbers would just go to the max allowed length in most cases. |
Hey I wanted to ask about the performance issues and see if there is any way I can help. I am running this:
And getting this result:
This is happening when I run with num_beams=10 in the generate method. It feels strange that the model is struggling on simple cases like this. Is this an issue of model performance? Or perhaps something to do with the way we are masking tokens being too restrictive? Let me know your thoughts I have been stuck on this for a couple days trying to implement an SSN format and it really starts to struggle on comparable seemingly simple tasks. |
@JamesHill0 What mode are you using? It's a bit hard to say if that's the issue without knowing what the model is. All jsonformer can do is guarantee you get valid output and you did. num_beams also isn't going to do anything here because we already treat each option like a beam. You could try other models or another similar library like guidance or LMQL and see if either of those work better. |
I merged it in this branch, where I added probabilities too https://github.com/wassname/prob_jsonformer also I made a list of other libs here https://github.com/wassname/awesome-interpretability/tree/main?tab=readme-ov-file#structured-output |
This PR adds 2 new types and enforces their correct generation:
Example schema:
I also went ahead and fixed the issue described in the todo in the generate bool method since it was an easy fix.
BTW, the performance is much better since I last checked up on this library so great job on that!