search

facebookresearch / EmpatheticDialogues

Dialogue model that produces empathetic responses when trained on the EmpatheticDialogues dataset.
Other
444 stars 63 forks source link

Logical bug? #47

Closed nizish closed 2 years ago

nizish commented 2 years ago

This code looks illogical.

If new_opt doesn't have fasttext set new_opt.fasttext on saved_opt.

if not (hasattr(new_opt, "fasttext")):
    setattr(saved_opt, "fasttext", new_opt.fasttext)

https://github.com/facebookresearch/EmpatheticDialogues/blob/9649114c71e1af32189a3973b3598dc311297560/empchat/models.py#L70

EricMichaelSmith commented 2 years ago

Hmm, yeah, that looks very fishy - it's been a few years since I've used this code, but I imagine that "fasttext" was never missing from new_opt or something and so maybe we didn't notice this issue. Is it a blocker for you being able to use this code?

nizish commented 2 years ago

No, as I went thru the code I found it. There were more patterns like this.

EricMichaelSmith commented 2 years ago

Okay, thanks for letting me know this - I don't have bandwidth to make and test a patch myself, but it's good to know this for others who encounter this issue!