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

[Empathetic Dialogues] Switch to DialogTeacher #4405

Merged
merged 31 commits into from Mar 15, 2022

Conversation

EricMichaelSmith
Copy link
Contributor

@EricMichaelSmith EricMichaelSmith commented Mar 8, 2022

Patch description

  • Switch EmpatheticDialogues over to DialogTeacher to allow for efficient eval of this dataset in distributed mode
  • Delete an unused teacher and unused flags and Message keys
  • Also switch EDPersonaTopicifierTeacher, a version of EmpatheticDialogues that includes personas, over to DialogTeacher

Testing steps

  • Checked that the following pass:
    • pytest tests/tasks/test_empathetic_dialogues.py
    • pytest parlai/tasks/empathetic_dialogues/test.py
    • pytest parlai/tasks/blended_skill_talk/test.py
    • pytest tests/tasks/test_style_gen_teachers.py
  • Compared the output of parlai dd -t empathetic_dialogues -v and parlai dd -t parlai.tasks.blended_skill_talk.agents:EDPersonaTopicifierTeacher before and after this PR

@EricMichaelSmith EricMichaelSmith changed the title [WIP] [Empathetic Dialogues] Switch to DialogTeacher [Empathetic Dialogues] Switch to DialogTeacher Mar 11, 2022
@EricMichaelSmith EricMichaelSmith marked this pull request as ready for review March 11, 2022 15:14
Copy link
Contributor

@stephenroller stephenroller left a comment

Choose a reason for hiding this comment

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

very nice! great cleanup and faster to boot!

parlai/tasks/empathetic_dialogues/agents.py Outdated Show resolved Hide resolved
parlai/tasks/empathetic_dialogues/agents.py Outdated Show resolved Hide resolved
experiencer_text_dialogue, responder_text_dialogue
)

for episode in data:
Copy link
Contributor

Choose a reason for hiding this comment

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

in an ideal world we would make this entire function an iterator that didn't explicitly load data into memory, which speeds up parlai dd and lowers peak memory usage.

it's fine for this PR

@stephenroller
Copy link
Contributor

Make sure you merge in master to get the fixes to CI, and check pytest parlai/tasks/empathetic_dialogues/test.py (which you seemed to skip since it has two sets of tests)

@EricMichaelSmith
Copy link
Contributor Author

Make sure you merge in master to get the fixes to CI, and check pytest parlai/tasks/empathetic_dialogues/test.py (which you seemed to skip since it has two sets of tests)

Good point, checked

@EricMichaelSmith EricMichaelSmith merged commit 4f7b38e into main Mar 15, 2022
@EricMichaelSmith EricMichaelSmith deleted the ed-teacher-overhaul branch March 15, 2022 16:45


class EmpatheticDialoguesTeacher(FixedDialogTeacher):

Choose a reason for hiding this comment

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

ImportError: cannot import name 'EmpatheticDialoguesTeacher' from 'parlai.core.teachers'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you paste the command that gives that error?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants