-
Notifications
You must be signed in to change notification settings - Fork 511
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 logits method to Sequence class #237
Conversation
I think it would make more sense to return the logprob as part of the Then we could create a custom object that contains both the text output and the logprob. This might do for now, give me some time to think about it. What are you using this for? |
If you include it in the
It's just very useful to be able to analyse the next token logits to e.g. assess the confidence of the model on a True/False question for various questions (and this pairs very well with regex). |
Ah I see what you mean, It would be useful to have an additional method with a simple API like I.e. returns the next token logits or probs without actually doing any sampling. |
Ok I've thought about this a bit more. IMO the best API would be to have a With this new function it may also make sense to have Was there a specific reason to have WDYT? @rlouf @brandonwillard |
Unnecessary anticipation of SMC sampling.
Sounds good to me. |
Do you prefer I can imagine use cases where the user wants both |
|
This reverts commit 8d85119.
I'm now thinking that indeed the Let's leave |
I've had a look at this and I'm not sure it's needed. The pros:
The cons:
@rlouf - Is it possible to consistently work with logits rather than probs, just for numerical stability. I've had a look at the code and it's not clear to me why you're not using torch.multinomial |
(Also sorry @rlouf I must've mis-clicked. I don't think I meant to remove you as a reviewer) |
Solved in #366 |
Allows the user to access the next token logits for a given prompt (including with regex!)