-
Notifications
You must be signed in to change notification settings - Fork 399
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
Fix some typos/language in regex
and sequence
#257
Conversation
outlines/text/generate/regex.py
Outdated
|
||
Parameters | ||
--------- | ||
model | ||
The model to use to computes the next-token logits. | ||
The model to use to compute the next-token logits. |
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.
This is a nitpick, but I think these comments for model
args might be more welcoming to some readers with less context, without losing clarity, if we said "the language model" / "the text transformer model" or something, instead of just "the model" - being that "model" is a very vague word and requires previous context to understand. For example, just in this file alone, there is a connotation of "model" as a text transformer model and "model" as a Pydantic data validation model.
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.
Hey @jrysana, that sounds like a good idea to me 🙂 would you want to make those changes?
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.
Sure, pushed @zaqqwerty
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.
Hi @jrysana, thank you for the PR! We appreciate your thorough reading of the code. LGTM, lmk if you'd first like to update based on your suggestion before we merge.
outlines/text/generate/regex.py
Outdated
|
||
Parameters | ||
--------- | ||
model | ||
The model to use to computes the next-token logits. | ||
The model to use to compute the next-token logits. |
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.
Hey @jrysana, that sounds like a good idea to me 🙂 would you want to make those changes?
Please correct me if I'm wrong about any of the below! But I noticed a few more things that seem mismatched in comments.
regex/create_proposal
implying that this method is only for integer generation, whereas it seems clear that this method is used for all schemas.README.md