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

Add new HRED agent and integration tests #2989

Merged
merged 8 commits into from
Aug 28, 2020

Conversation

NathanShuster
Copy link
Contributor

Patch description

Implements a basic HRED agent agent that extends seq2seq. I originally implemented a heavier version that had a lot of dataset specific features for SIMMC and stripped out all the dataset specific stuff for this PR. Is a general implementation but has some room for added functionality (for example, currently only supports LSTM, not GRU) that is not in this PR.

Testing steps
As part of my intern project I reproduced the SIMMC (https://github.com/facebookresearch/simmc) HRED baseline in ParlAI, and successfully reproduced the original results. To generalize for broader use in ParlAI, I stripped out all the dataset specific features and consulted with Stephen in the process. I ran some follow up testing with the DailyDialog set, shared my findings with Stephen and my manager, and the numbers were deemed sufficiently close to send out a PR.

Logs
Can be run with any of the ParlAI datasets with -m seq2seq/hred

parlai/agents/seq2seq/hred.py Outdated Show resolved Hide resolved
parlai/agents/seq2seq/hred.py Outdated Show resolved Hide resolved
@NathanShuster NathanShuster changed the title Add HRED agent that extends seq2seq Add new HRED agent and integration tests Aug 26, 2020
@stephenroller
Copy link
Contributor

Test that is failing is that you need to add an __init__.py into your hred folder, with its only contents being the shebang and copyright comment

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.

Thanks Nathan!

@stephenroller
Copy link
Contributor

Tests will pass on master, depended on #3019.

@stephenroller stephenroller merged commit e08125c into facebookresearch:master Aug 28, 2020
@stephenroller stephenroller mentioned this pull request Sep 5, 2020
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.

3 participants