-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[BlenderBot2] clean up reply during interactive mode #4379
Conversation
""" | ||
|
||
if ( | ||
self.opt('clean_reply', False) |
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.
So it returns here and doesn't go to cleaning the text if clean_reply
is True
?
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'd love to ALWAYS clean the reply and I think it's the right thing to do?
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'd love to ALWAYS clean the reply and I think it's the right thing to do?
could you maybe add some test cases to make sure that |
if self_message is not None: | ||
last_reply = self_message['text'] | ||
clean_reply = self._clean_text(last_reply) | ||
self.history.add_reply(clean_reply) |
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.
perhaps we should just override the History class and do this logic there?
Go ahead and merge main into this branch |
Patch description
Right now, during interactive mode, the
'_POTENTIALLY_UNSAFE__'
from blenderbot2 generation would go into blenderbot 2 history and possibly affects its subsequent generation. However the model is never trained on observing such specially tokens, only generating them.After the change, we still keep the
'_POTENTIALLY_UNSAFE__'
in the reply text, and only remove that from the blenderbot2 history.Testing steps
Other information