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.

Sign up for GitHub

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

Jump to bottom

support both old and new fastText model #1319

Merged
merged 19 commits into from May 25, 2017
Merged

support both old and new fastText model #1319

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
9f9dd24
mismatch in bin and vec file resolved
prakhar2b May 13, 2017
d7725ca
support old and new format
prakhar2b May 19, 2017
de39ab0
handling whitespace
prakhar2b May 19, 2017
f0c3e25
support old and new format
prakhar2b May 19, 2017
5f5ace6
branch conflict with resolved
prakhar2b May 19, 2017
1509512
support old and new fastText format
prakhar2b May 20, 2017
d7e5403
model trained with new fastText format
prakhar2b May 20, 2017
58a66c2
test for loading new fastText format
prakhar2b May 20, 2017
9c9d3ec
Merge branch 'develop' into mismatch
prakhar2b May 22, 2017
8ffb220
french PR separated
May 22, 2017
06ac316
magic header separated
May 22, 2017
3deb394
changes according to review
May 22, 2017
b038fdb
.vec added to flake8 ignore list
prakhar2b May 23, 2017
4f6aa4d
self.new_format mistake corrected
prakhar2b May 23, 2017
5c09bdf
localBounderror resolved
prakhar2b May 23, 2017
5cdf4e6
Merge branch 'develop' of https://github.com/RaRe-Technologies/gensim…
May 23, 2017
55a2d37
unused variable error handled
May 23, 2017
aeb05c1
two space before comment
May 23, 2017
092ef86
Merge branch 'develop' into mismatch
prakhar2b May 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 26 additions & 10 deletions gensim/models/wrappers/fasttext.py
View file
Open in desktop
Expand Up @@ -140,6 +140,7 @@ class FastText(Word2Vec):

def initialize_word_vectors(self):
self.wv = FastTextKeyedVectors()
self.new_format = False

@classmethod
def train(cls, ft_path, corpus_file, output_file=None, model='cbow', size=100, alpha=0.025, window=5, min_count=5,
Expand Down Expand Up @@ -256,7 +257,14 @@ def load_binary_data(self, model_binary_file):
self.load_vectors(f)

def load_model_params(self, file_handle):
(dim, ws, epoch, minCount, neg, _, loss, model, bucket, minn, maxn, _, t) = self.struct_unpack(file_handle, '@12i1d')
magic, v= self.struct_unpack(file_handle, '@2i')
Copy link
Contributor

@jayantj jayantj May 22, 2017 •
edited

Choose a reason for hiding this comment

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

Code style - keep 1 space before and after =
Also, version would be preferable to v.

if magic == 793712314: # newer format
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be good to store this value in a global variable. FASTTEXT_MAGIC_HEADER or something similar.

self.new_format = True
dim, ws, epoch, minCount, neg, _, loss, model, bucket, minn, maxn, _, t = self.struct_unpack(file_handle, '@12i1d')
else: # older format
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to set self.new_format = False inside this else, rather than inside initialize_word_vectors

dim = magic
ws = v
epoch, minCount, neg, _, loss, model, bucket, minn, maxn, _, t = self.struct_unpack(file_handle, '@10i1d')
# Parameters stored by [Args::save](https://github.com/facebookresearch/fastText/blob/master/src/args.cc)
self.size = dim
self.window = ws
Expand All @@ -270,26 +278,34 @@ def load_model_params(self, file_handle):
self.wv.max_n = maxn
self.sample = t

def load_dict(self, file_handle):
(vocab_size, nwords, _) = self.struct_unpack(file_handle, '@3i')
def load_dict(self, file_handle, encoding='utf8'):
vocab_size, nwords, _ = self.struct_unpack(file_handle, '@3i')
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer keeping the changes related to the issue with the french wiki in a separate PR. We don't want those changes to block this PR from being merged.
In general, I think it's good practice to keep fixes for different issues in different branches/PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

separated the PRs. Thanks :)

# Vocab stored by [Dictionary::save](https://github.com/facebookresearch/fastText/blob/master/src/dictionary.cc)
assert len(self.wv.vocab) == nwords, 'mismatch between vocab sizes'
assert len(self.wv.vocab) == vocab_size, 'mismatch between vocab sizes'
ntokens, = self.struct_unpack(file_handle, '@q')
if len(self.wv.vocab) != vocab_size:
logger.warnings("If you are loading any model other than pretrained vector wiki.fr, ")
logger.warnings("Please report to gensim or fastText.")
ntokens= self.struct_unpack(file_handle, '@1q')
if self.new_format:
pruneidx_size = self.struct_unpack(file_handle, '@q')
Copy link
Contributor

Choose a reason for hiding this comment

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

pruneidx_size seems to be used in the original FastText code to load dictionaries -

void Dictionary::load(std::istream& in) {
  words_.clear();
  std::fill(word2int_.begin(), word2int_.end(), -1);
  in.read((char*) &size_, sizeof(int32_t));
  in.read((char*) &nwords_, sizeof(int32_t));
  in.read((char*) &nlabels_, sizeof(int32_t));
  in.read((char*) &ntokens_, sizeof(int64_t));
  in.read((char*) &pruneidx_size_, sizeof(int64_t));
  for (int32_t i = 0; i < size_; i++) {
    char c;
    entry e;
    while ((c = in.get()) != 0) {
      e.word.push_back(c);
    }
    in.read((char*) &e.count, sizeof(int64_t));
    in.read((char*) &e.type, sizeof(entry_type));
    words_.push_back(e);
    word2int_[find(e.word)] = i;
  }
  pruneidx_.clear();

  for (int32_t i = 0; i < pruneidx_size_; i++) {
    int32_t first;
    int32_t second;
    in.read((char*) &first, sizeof(int32_t));
    in.read((char*) &second, sizeof(int32_t));
    pruneidx_[first] = second;
  }

  initTableDiscard();
  initNgrams();
}

I'm not sure if it's present in the models we're loading or not though?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for j in range(pruneidx_size):  
            _,_ = self.struct_unpack(file_handle,'@2i')

Presence (or absence) of these two lines doesn't affect the code.

Copy link
Contributor

@jayantj jayantj May 22, 2017 •
edited

Choose a reason for hiding this comment

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

I don't see those two lines in the code. Do you mean pruneidx_size is always zero, and it doesn't matter whether we keep those lines or not?
If yes, we should add an assert statement, since if pruneidx_size isn't zero and we don't read the corresponding bytes, the subsequent bytes are not going to be parsed correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's -1 actually. Added these two lines in the new commit. I think we should not use assert statement here as some models might have non-negative values, so adding these two lines should be sufficient.

for i in range(nwords):
word_bytes = b''
char_byte = file_handle.read(1)
# Read vocab word
while char_byte != b'\x00':
word_bytes += char_byte
char_byte = file_handle.read(1)
word = word_bytes.decode('utf8')
count, _ = self.struct_unpack(file_handle, '@ib')
_ = self.struct_unpack(file_handle, '@i')
assert self.wv.vocab[word].index == i, 'mismatch between gensim word index and fastText word index'
self.wv.vocab[word].count = count
word = word_bytes.decode(encoding)
count, _ = self.struct_unpack(file_handle, '@qb')
if word in self.wv.vocab:
# skip loading info about words in bin file which are not present in vec file
# handling mismatch in vocab_size in vec and bin files (ref: wiki.fr)
assert self.wv.vocab[word].index == i, 'mismatch between gensim word index and fastText word index'
self.wv.vocab[word].count = count

def load_vectors(self, file_handle):
if self.new_format:
_ = self.struct_unpack(file_handle,'@?')
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add a comment clarifying what this is for?

num_vectors, dim = self.struct_unpack(file_handle, '@2q')
# Vectors stored by [Matrix::save](https://github.com/facebookresearch/fastText/blob/master/src/matrix.cc)
assert self.size == dim, 'mismatch between model sizes'
Expand Down
Binary file added gensim/test/test_data/lee_fasttext_new.bin
View file
Open in desktop
Binary file not shown.