Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dictionary entries with #fairseq:overwrite are not preserved in dict.txt output from fairseq-preprocess #3705

Open
nelson-liu opened this issue Jul 11, 2021 · 1 comment · May be fixed by #5329

Comments

@nelson-liu
Copy link
Contributor

🐛 Bug

To Reproduce

Steps to reproduce the behavior (always include the command you ran):

  1. Run fairseq-preprocess with a --srcdict that has #fairseq:overwrite. For example, the command in the roberta pretraining tutorial https://github.com/pytorch/fairseq/blob/master/examples/roberta/README.pretraining.md#1-preprocess-the-data
  2. Look at the dictionary that is output by fairseq-preprocess, and see that #fairseq:overwrite is not preserved in dict.txt.

Expected behavior

When using --srcdict, the dict.txt should be exactly the same as the one passed in to fairseq-preprocess

Environment

  • fairseq Version (e.g., 1.0 or master): 0.10.2
  • PyTorch Version (e.g., 1.0) 1.9.0
  • OS (e.g., Linux): Linux
  • How you installed fairseq (pip, source): pip
@lydianish
Copy link

Hi @nelson-liu ,

I think the bug comes from the add_symbol function in fairseq/fairseq/data/dictionary.py file:

def add_symbol(self, word, n=1, overwrite=False):
        """Adds a word to the dictionary"""
        if word in self.indices and not overwrite:
            idx = self.indices[word]
            self.count[idx] = self.count[idx] + n
            return idx
        else:
            idx = len(self.symbols)
            self.indices[word] = idx
            self.symbols.append(word)
            self.count.append(n)
            return idx

The condition should be changed to if word in self.indices and overwrite:. Otherwise, even when overwrite is indeed set to True, the symbols will not be overwritten.

In your case, this results the dictionary having duplicate special tokens. Normally, when loading from file, the dictionary should start with 4 special tokens (<s>, <pad>, </s> and <unk> in that order), followed by the entries in the file. If your file already has special tokens with #fairseq:overwrite, they should be overwritten. The dict.txt saved during preprocessing is meant to skip the those first tokens, and should not have those #fairseq:overwrite tags.

I have created a pull request that fixes the bug (#5329).

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

Successfully merging a pull request may close this issue.

2 participants