CMU_DoG

[spencerp]

Patch description
Adding CMU_DoG dataset. This is mostly a copypasta of code that wasn't yet checked in, and was originally written mostly by Moya and myself.

Testing steps

λ parlai dd -t cmu_dog
λ parlai dd -t cmu_dog --cmu-dog-split-type original
λ parlai dd -t cmu_dog --cmu-dog-split-type seen --datatype test
λ parlai dd -t cmu_dog --cmu-dog-split-type unseen --datatype test
λ parlai dd -t cmu_dog --cmu-dog-include-knowledge-keys movieName
λ parlai dd -t cmu_dog --cmu-dog-provide-movie-context False --mutators knowledge_only_when_updated,prepend_knowledge_to_message
λ parlai dd -t cmu_dog --cmu-dog-only-with-knowledge False --cmu-dog-rating 1 --cmu-dog-multi-msg-delimiter "__" --cmu-dog-fact-delimiter "/"

Unit tests

λ pytest -k test_zootasks
λ pytest --force-regen parlai/tasks/cmu_dog/test.py  # fails initially, as expected
λ pytest parlai/tasks/cmu_dog/test.py  # succeeds

[moyapchen]

Thanks for putting this up!

[moyapchen]

[No-op, just a comment] As a sidenote,

list(train_seen + valid_seen + test_seen + test_unseen) = list(train_original_deduped + valid_original_deduped + test_original_deduped)

so we could do that... but the code is probably clearer this way.

(On that note, this does mean that we will be duplicate counting conversations when we do F1 on the original split, but it seems like we're deciding to be okay with that.)

[spencerp]

list(train_seen + valid_seen + test_seen + test_unseen) = list(train_original_deduped + valid_original_deduped + test_original_deduped)

I'm leaning toward leaving these separate for code clarity. But it might be good to add this relationship as a unit test so that it's documented.

we will be duplicate counting conversations when we do F1 on the original split

When you say "original split" you mean the one that isn't deduplicated, right? I guess I'm kind of assuming that if someone is intentionally using the split with duplicate conversations, it's probably desirable that the conversations be duplicated in the metric calculations as well.

Maybe I should add a warning print if people use the original split type, and point them to your issue (festvox/datasets-CMU_DoG#2) so they know what they're getting into.

[klshuster]

agree with what you say spencer - yes, if we're providing the original split, we should calculate metrics on the duplicates as well. the warning below looks good!

[klshuster]

thanks! see comments, though as this is merged i think the build comment is the only one necessary to address (others are mostly nits)

[klshuster]

agree with what you say spencer - yes, if we're providing the original split, we should calculate metrics on the duplicates as well. the warning below looks good!

[klshuster]

nit: as this defaults to True, and is presumably the desired arg, might be good to update the help comment here, like "Set False to optionally train both sides of conversation"

[klshuster]

should we make movieName be the default here? or at least the recommended? as at least for our future experimentation we'll like to use that split.

alternatively, maybe we can provide a teacher, like MovieNameTeacher, which sets this arg for you (so you can do something like parlai dd -t cmu_dog:movie_name and it'll work appropriately)

[klshuster]

messaged offline about this but we should be downloading the dataset from the source, and building the splits ourselves (in the build function below)