-
Notifications
You must be signed in to change notification settings - Fork 175
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
[TextGeneration] Add GeneratedText
and update TextGenerationOutput
#1240
Conversation
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.
LGTM overall - let's check any downstream usages or tests of the pipeline that changing the output like this may break
LGTM, let's add some testing and make sure that it doesn't break things downsteam (perplexity computation). |
115a7d7
to
6a79bca
Compare
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.
Nice
Summary
Add
GeneratedText
schema to theTextGeneration
pipelineUpdate the
TextGenerationOutput
to use theGeneratedText
class for each output sequenceUse the
FinishReason
enum for the logged reason for generation to complete. Question - missing reason for any generic callback?Following internal docs: https://github.com/neuralmagic/internal-docs/blob/0bd3bf122c1f8c5871b085916d04cb0266a0a332/requirements/deepsparse/natural-language-generation/main.md
One missing piece in in the
GeneratedText
is the use of aliasAlso, we're keeping the
prompt
in theTextGenerationOutput
as opposed to the correspondingGeneratedText
which seems counterintuitive?Testing
First Generation
[GeneratedText(text=' color is the willow fern?\nI got it from Panera years ago. Will do a full review of this Matt veg rose', score=None, finished=True, finished_reason='stop token generated.'), GeneratedText(text=" are your plans for your website?\nI was in the process of completing that actually. I mean, I'm probs not going to add more features like a hub video or anything but as far as making the website look prettier it's a work in progress. Anyways, by the look of the username I'm guessing you have some contacts in the film world? Want to work on something together?\nNot sure. LuckyIknew.com isn't exactly sparkling when I tried to navigate through it just now. Uhm... When are you going to put together all the footage and do it so the audio matches up with the footage?\nI'll do that now. I've had to go to my basement for a few weeks though, all the footage reels are in there. Will happen as soon as I can get back in there and redo it properly. Short of a whole bunch of stuff happening, I won't be able to put too much effort into it, sadly.", score=None, finished=True, finished_reason='stop token generated.')]
Second Generation
[GeneratedText(text=' houdini dragon edition? Oh every1 is spelling honey a weird way.... THE HELL DO YOU MEAN?\nMy word is brown?', score=None, finished=True, finished_reason='stop token generated.'), GeneratedText(text="!! hello?! where's her obsession with Yugoslavs?\nthe Yugoslavian attitudes were everyone stands still, only steps on the yard is path, slowly walks to the other side returns, dont, coulor-moning... all with a mix of 'joking' and half serious and i think thats why i like it", score=None, finished=True, finished_reason='stop token generated.')]