Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

[BB2] LTM Mutator #3966

Merged
merged 1 commit into from
Sep 13, 2021
Merged

[BB2] LTM Mutator #3966

merged 1 commit into from
Sep 13, 2021

Conversation

klshuster
Copy link
Contributor

Patch description
Adds the Long-Term-Memory mutator for multi-tasked training of the Query Generator in BB2. Thus, one could recreate training of the query generator via --task wizard_of_internet:SearchQueryTeacher,msc:mutators=ltm_mutator

Testing steps
Verified via parlai dd and via new test

$ parlai dd -t msc --mutators ltm_mutator
.
.
.
- - - NEW EPISODE: msc:Session1Self - - -
your persona: I like to remodel homes.
your persona: I like to go hunting.
your persona: I like to shoot a bow.
your persona: My favorite holiday is halloween.
Hi, how are you doing? I'm getting ready to do some cheetah chasing to stay in shape.
   personal_knowledge

@klshuster klshuster requested a review from jxmsML August 20, 2021 20:39
@klshuster klshuster mentioned this pull request Aug 20, 2021
Closed
jxmsML
jxmsML reviewed Sep 9, 2021
View reviewed changes
parlai/tasks/msc/mutators.py
def many_episode_mutation(self, episode: List[Message]) -> List[List[Message]]:
history = []
for message in episode:
if message['text'] == '__SILENCE__':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one nit: replace this '__SILENCE__' with DUMMY_TEXT

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy to do this, i think i hesitated at first because it would require a nested import (to avoid circular dependencies), but should be fine for me!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah didn't realize it would require a nested import. I'm fine either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, if you're ok with it i'll avoid the nested import for now, then

jxmsML
jxmsML approved these changes Sep 9, 2021
View reviewed changes
Copy link
Contributor

@jxmsML jxmsML left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

@klshuster klshuster merged commit 136ffaa into main Sep 13, 2021
@klshuster klshuster deleted the msc_mutator branch September 13, 2021 17:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jxmsML jxmsML jxmsML approved these changes

Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@klshuster @jxmsML @facebook-github-bot