-
Notifications
You must be signed in to change notification settings - Fork 144
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
Merge Search & State into new "Generator" type #39
Conversation
RyanUnderhill
commented
Jan 26, 2024
# input_tokens = tokenizer.encode(text, return_tensors='np') | ||
input_tokens = tokenizer.encode(text) | ||
input_tokens = tokenizer.encode(text, return_tensors='np') | ||
# input_tokens = tokenizer.encode(text) | ||
|
||
params=og.SearchParams(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.
could you please also change SearchParams to GeneratorParams?
|
||
# search.Apply_MinLength(1) | ||
# search.Apply_RepetitionPenalty(1.0) | ||
|
||
search.SampleTopP(0.7, 0.6) | ||
generator.AppendNextToken_TopP(0.7, 0.6) |
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.
Append doesn't look like a good name to me. generator actually generate next token. How about GenerateNextToken
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.
And can we make TopP a parameter instead of part of the name?
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.
I named it that since you first compute the logits, then append the next token based on the logits.
I initially had AddNextToken but 'Add' was less correct than 'Append' since it's appending tokens, not adding them.
Did you have a better name?
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.
It is a process of generate next token. Add or Append doesn't reflect the action, i think.
# search.Apply_MinLength(1) | ||
# search.Apply_RepetitionPenalty(1.0) |
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.
could you please update this also