-
Notifications
You must be signed in to change notification settings - Fork 93
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 multinomial sampling to sequence generation #161
Conversation
lib/bumblebee/text/generation.ex
Outdated
|
||
* `:sample` - whether or not to use random sampling. Defaults to `false` | ||
|
||
* `:prng_key` - random key to use when sampling. Defaults to `nil` |
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.
Since this function is pretty higher-level, what about :seed
instead?
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.
Agreed, though one thing I was thinking was if we should pass this in the serving as well and return the updated key. They do accept key
in generate
in HF, but they do not return an updated key, so maybe just allowing a key to the serving is enough and it is assumed you will split it outside the serving
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.
Servings are so high-level that it seems to me the seed is for reproducibility if anything else (or to explore different result for the same prompt), and so a number is more transparent than a tensor key. Or do you think there's a scenario where you would actually use a split key?
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.
Sorry, what I mean is currently you'd need to create a new serving with a new seed to get different inputs. Maybe this is desirable though, but I'm not sure as if you want something non-deterministic you may want the option to pass a key to the serving
Co-authored-by: José Valim <jose.valim@dashbit.co>
Co-authored-by: José Valim <jose.valim@dashbit.co>
2ad5aad
to
fbd10ce
Compare
@seanmor5 I finished sampling, we can do beam search separately, especially that we have contrastive search already :) |
If you think about it, every search we do is a beam search. |
Adds multinomial sampling strategy and top-k/top-p options for adjusting the distribution.